Reduce locking when persisting ML job statistics. by jan-elastic · Pull Request #141519 · elastic/elasticsearch

jan-elastic · 2026-01-29T14:09:41Z

fixes #140511

elasticsearchmachine · 2026-01-29T14:10:09Z

Pinging @elastic/ml-core (Team:ML)

elasticsearchmachine · 2026-01-29T14:10:09Z

Hi @jan-elastic, I've created a changelog YAML for you.

elasticsearchmachine · 2026-01-29T14:31:36Z

Hi @jan-elastic, I've updated the changelog YAML for you.

jan-elastic · 2026-01-30T07:26:13Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/DataCountsReporter.java


-    public synchronized DataCounts runningTotalStats() {
+    public DataCounts runningTotalStats() {
        totalRecordStats.calcProcessedFieldCount(getAnalysedFieldsPerRecord());


Note: this "update the object upon read" is a bit strange.

I've considered to:

move analysedFieldsPerRecord to DataCounts

make the calcProcessedFieldCount private

call it when you call setXyz() (Xyz is something that influences processFieldCount)

What do you think about this?

I didn't do it, because I wanted the changes to be as minimal as possible.

We don't make this change because this would increase the overhead of the method reportRecordWritten() by about 20%. This method lies on the hot path and is executed thousands of times per minute. It should be kept as fast as possible, even at the cost of a more cumbersome design.

valeriy42 · 2026-01-29T14:54:45Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/DataCountsReporter.java

-                synchronized (this) {
-                    unreportedStats = statsToReport;
-                }
+                unreportedStats.set(statsToReport);


The following race scenario between finishReporting and writeUnreportedCounts is possible:

Thread A (finishReporting):

1. unreportedStats.set(null) // Clear unreported stats 2. statsToReport = runningTotalStats() 3. persistDataCounts(..., false) → returns false (previous persist in progress)

Thread B (writeUnreportedCounts):

4. getAndSet(null) → returns null // Sees null, does nothing

Thread A:

5. unreportedStats.set(statsToReport) // Sets stats that should have been persisted

Result: Stats are in unreportedStats but won't be persisted until the next writeUnreportedCounts() call

This is not very bad, since the next flushJob() will persist them (datafeeds flush regularly, typically every few minutes). But it's probably worth documenting at this point that we guarantee only eventual consistency.

valeriy42 · 2026-01-30T08:01:41Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/DataCountsReporter.java


-    public synchronized DataCounts runningTotalStats() {
+    public DataCounts runningTotalStats() {
        totalRecordStats.calcProcessedFieldCount(getAnalysedFieldsPerRecord());


We don't make this change because this would increase the overhead of the method reportRecordWritten() by about 20%. This method lies on the hot path and is executed thousands of times per minute. It should be kept as fast as possible, even at the cost of a more cumbersome design.

jan-elastic

Unit test LGTM.

valeriy42

LGTM. Thank you for fixing this!

* Reduce locking when persisting ML job statistics. * Update docs/changelog/141519.yaml * Update docs/changelog/141519.yaml * add unit test * improved timeout exception handling. * count down latch on exception * fix javadoc in unittest * [CI] Auto commit changes from spotless --------- Co-authored-by: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

elasticsearchmachine · 2026-01-30T13:02:44Z

💚 Backport successful

Status	Branch	Result
✅	9.3
✅	8.19
✅	9.2

* Reduce locking when persisting ML job statistics. * Update docs/changelog/141519.yaml * Update docs/changelog/141519.yaml * add unit test * improved timeout exception handling. * count down latch on exception * fix javadoc in unittest * [CI] Auto commit changes from spotless --------- Co-authored-by: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…141630) Merging patch/serverless-fix back to main

jan-elastic added >bug :ml Machine learning Team:ML Meta label for the ML team v9.4.0 labels Jan 29, 2026

jan-elastic requested a review from valeriy42 January 29, 2026 14:09

jan-elastic commented Jan 30, 2026

View reviewed changes

valeriy42 reviewed Jan 30, 2026

View reviewed changes

valeriy42 self-requested a review January 30, 2026 08:52

jan-elastic force-pushed the reduce-DataCountsReporter-locking branch from bcf196d to 8579849 Compare January 30, 2026 08:57

jan-elastic commented Jan 30, 2026

View reviewed changes

valeriy42 approved these changes Jan 30, 2026

View reviewed changes

jan-elastic and others added 6 commits January 30, 2026 11:38

Reduce locking when persisting ML job statistics.

3a78417

Update docs/changelog/141519.yaml

7155273

Update docs/changelog/141519.yaml

0c1958c

add unit test

3caf72e

improved timeout exception handling.

4917365

count down latch on exception

1c74a01

jan-elastic force-pushed the reduce-DataCountsReporter-locking branch from 1f2078f to 1c74a01 Compare January 30, 2026 10:38

valeriy42 and others added 2 commits January 30, 2026 11:45

fix javadoc in unittest

54b884b

[CI] Auto commit changes from spotless

959d8fa

valeriy42 added auto-backport Automatically create backport pull requests when merged v9.3.1 v8.19.12 v9.2.6 labels Jan 30, 2026

jan-elastic merged commit 726299b into elastic:main Jan 30, 2026
35 checks passed

This was referenced Jan 30, 2026

[9.3] Reduce locking when persisting ML job statistics. (#141519) #141583

Merged

[8.19] Reduce locking when persisting ML job statistics. (#141519) #141584

Merged

jan-elastic mentioned this pull request Jan 30, 2026

[9.2] Reduce locking when persisting ML job statistics. (#141519) #141585

Merged

breskeby added a commit that referenced this pull request Feb 1, 2026

[Merge] Reduce locking when persisting ML job statistics. (#141519) (#…

d640fc0

…141630) Merging patch/serverless-fix back to main

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce locking when persisting ML job statistics.#141519

Reduce locking when persisting ML job statistics.#141519
jan-elastic merged 8 commits intoelastic:mainfrom
jan-elastic:reduce-DataCountsReporter-locking

jan-elastic commented Jan 29, 2026 •

edited

Loading

elasticsearchmachine commented Jan 29, 2026

elasticsearchmachine commented Jan 29, 2026

elasticsearchmachine commented Jan 29, 2026

jan-elastic Jan 30, 2026

valeriy42 Jan 30, 2026 •

edited

Loading

valeriy42 Jan 29, 2026

valeriy42 Jan 30, 2026 •

edited

Loading

jan-elastic left a comment

valeriy42 left a comment

Uh oh!

elasticsearchmachine commented Jan 30, 2026

Labels

3 participants

Conversation

jan-elastic commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Jan 29, 2026

elasticsearchmachine commented Jan 29, 2026

elasticsearchmachine commented Jan 29, 2026

jan-elastic Jan 30, 2026

Choose a reason for hiding this comment

valeriy42 Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

valeriy42 Jan 29, 2026

Choose a reason for hiding this comment

valeriy42 Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

jan-elastic left a comment

Choose a reason for hiding this comment

valeriy42 left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jan 30, 2026

💚 Backport successful

Labels

3 participants

jan-elastic commented Jan 29, 2026 •

edited

Loading

valeriy42 Jan 30, 2026 •

edited

Loading

valeriy42 Jan 30, 2026 •

edited

Loading