apache / apache/doris

[Bug](profile) ProfileManager leaks profile-loader threads when the profile IO executor is saturated or cold-load fails

Open
#66,311 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
15.9k
Forks
3.9k
Avg merge
2d 23h
Merged PRs (30d)
520

Description

### Search before asking

- [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues.

### Version

master

### What's Wrong?

On the FE, `ProfileManager.loadProfilesFromStorageIfFirstTime(boolean sync)` (the cold-load path that reads persisted query profiles from disk into memory) has two problems that can leak `profile-loader` threads:

1. **No in-flight guard.** Every call creates a brand-new `profile-loader` `Thread` with no check for a load already in progress. This method is reached from the periodic scheduler, so if a load has not yet finished a fresh loader thread is started on the next tick.

2. **`isProfileLoaded` is only set on success.** The flag `isProfileLoaded = true` is set *inside* the `try` block, *after* the batch loop completes. If the load throws, or the `profileIOExecutor` thread pool is saturated so the batch `future.get()` calls never return, the flag stays `false` forever. Combined with (1), a new `profile-loader` thread is then spawned on **every** scheduler tick — an unbounded thread leak that keeps growing until the FE is restarted.

```java
Thread loadThread = new Thread(() -> {
try {
... batch loop over profileIOExecutor futures ...
isProfileLoadedLock.writeLock().lock();
try {
this.isProfileLoaded = true; // only reached on success
} finally {
isProfileLoadedLock.writeLock().unlock();
}
} catch (Exception e) {
LOG.error("Failed to load query profile from storage", e); // flag left false
}
});
loadThread.setName("profile-loader");
loadThread.start();
```

### What You Expected?

At most one `profile-loader` thread should run at a time, and a cold-load that fails or never completes should not cause a new loader thread to be spawned on every scheduler tick.

### How to Reproduce?

Saturate the `profileIOExecutor` (or make `Profile.read`/`getOnStorageProfileInfos` block or throw) so `loadProfilesFromStorageIfFirstTime` never sets `isProfileLoaded = true`, then let the periodic scheduler keep calling it. The live `profile-loader` thread count grows without bound (observable via a thread dump / `Thread.getAllStackTraces`).

### Anything Else?

Fix: add an `AtomicBoolean isProfileLoading` CAS guard so only one loader runs at a time (synchronous callers block until the in-flight load finishes), and mark the load finished in a `finally` block **even on failure** so a saturated/failed load does not respawn a loader every second.

### Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)

Contributor guide

Open the contributing guide

Research direction

Start at ProfileManager.loadProfilesFromStorageIfFirstTime(boolean sync), then trace the periodic scheduler, profile-loader thread creation, and profileIOExecutor future handling. Reproduce the failure by saturating the executor or blocking profile reads, and verify that only one cold load runs at a time and failed or unfinished loads do not spawn a new loader on every scheduler tick.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.