Skip to content

Conversation

albertzaharovits
Copy link
Contributor

This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps).
This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks indices.merge.disk.watermark.high, and possibly consider implementing aborting already executing merges too.

Relates #88606 (comment)

@albertzaharovits albertzaharovits self-assigned this Jul 22, 2025
@albertzaharovits albertzaharovits added >enhancement :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v9.2.0 v9.1.1 v8.19.1 v9.0.5 labels Jul 22, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

@elasticsearchmachine
Copy link
Collaborator

Hi @albertzaharovits, I've created a changelog YAML for you.

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jul 22, 2025
@albertzaharovits
Copy link
Contributor Author

Here is a sample heap dump:
Capture d’écran 2025-07-22 à 19 33 30
It shows the merge tasks that are still taking up budget (i.e. disk space) (i.e. still running) and what the available budget was when the merge tasks were dequeued for scheduling.

assert super.lock.isHeldByCurrentThread();
Tuple<MergeTask, Long> head = enqueuedByBudget.peek();
if (head != null && head.v2() > availableBudget) {
LOGGER.warn("There are merge tasks enqueued but there's insufficient disk space available to execute them");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These log messages will be printed every indices.merge.disk.check_interval (5 sec). That sounds Ok to you, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful to include the availableBudget and head.v2() values in the log message too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, thanks for suggesting! Pushed: d334433

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me but I'm probably not best-placed to review this properly. Could you try someone from the distrib/indexing team instead?

@albertzaharovits
Copy link
Contributor Author

Thanks for looking, David!

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I left minor comments

assert super.lock.isHeldByCurrentThread();
Tuple<MergeTask, Long> head = enqueuedByBudget.peek();
if (head != null && head.v2() > availableBudget) {
LOGGER.warn("There are merge tasks enqueued but there's insufficient disk space available to execute them");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful to include the availableBudget and head.v2() values in the log message too?

Locale.ROOT,
"There are no merge tasks currently running, "
+ "but there are [%d] enqueued ones that are blocked because of insufficient disk space",
enqueuedByBudget.size()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, would it be useful to have the availableBudget and head.v2() values in the log message here?

messageBuilder.append(
"], and there are ["
+ enqueuedByBudget.size()
+ "] additional enqueued ones that are blocked because of insufficient disk space"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here too?

}
}

void postBudgetUpdate() {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assert super.lock.isHeldByCurrentThread(); here too

@albertzaharovits albertzaharovits added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Jul 23, 2025
@elasticsearchmachine elasticsearchmachine merged commit 9caa861 into elastic:main Jul 23, 2025
33 checks passed
@albertzaharovits albertzaharovits deleted the improve-unavailable-disk-space-diag branch July 23, 2025 15:17
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jul 23, 2025
…e merges (elastic#131711) This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps). This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks `indices.merge.disk.watermark.high`, and possibly consider implementing aborting already executing merges too. Relates elastic#88606 (comment)
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jul 23, 2025
…e merges (elastic#131711) This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps). This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks `indices.merge.disk.watermark.high`, and possibly consider implementing aborting already executing merges too. Relates elastic#88606 (comment)
elasticsearchmachine pushed a commit that referenced this pull request Jul 23, 2025
…e merges (#131711) (#131772) This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps). This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks `indices.merge.disk.watermark.high`, and possibly consider implementing aborting already executing merges too. Relates #88606 (comment)
elasticsearchmachine pushed a commit that referenced this pull request Jul 23, 2025
…e merges (#131711) (#131773) This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps). This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks `indices.merge.disk.watermark.high`, and possibly consider implementing aborting already executing merges too. Relates #88606 (comment)
elasticsearchmachine pushed a commit that referenced this pull request Jul 23, 2025
…e merges (#131711) (#131774) This goal of this PR is to record information about the available disk space at the time that merge tasks are scheduled (the info will show up in heap dumps). This should aid troubleshooting in cases where there are running merge tasks all while the available disk space on the node is below the watermark. In this case we should increase the threshold for scheduling merge tasks `indices.merge.disk.watermark.high`, and possibly consider implementing aborting already executing merges too. Relates #88606 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement Team:Distributed Indexing Meta label for Distributed Indexing team v8.19.1 v9.0.5 v9.1.1 v9.2.0

4 participants