Account for time taken to write index buffers in IndexingMemoryController #126786

ankikuma · 2025-04-14T15:45:33Z

Seeks to address ES-11356.

This PR adds to the indexing write load, the time taken to flush write indexing buffers using the indexing threads (this is done here to push back on indexing)

This changes the semantics of InternalIndexingStats#recentIndexMetric and InternalIndexingStats#peakIndexMetric to more accurately account for load on the indexing thread.

Refresh to latest

Refresh branch

…ch into 04142025/IWLES11356 pull

elasticsearchmachine · 2025-04-18T00:03:01Z

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

elasticsearchmachine · 2025-04-18T00:03:01Z

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

…ch into 04142025/IWLES11356 commit pull

fcofdez

Looks good, can we add a test where we exercise this and see how the write load increases when the IndexingMemoryController flushes the buffers to disk?

server/src/main/java/org/elasticsearch/indices/IndexingMemoryController.java

server/src/main/java/org/elasticsearch/index/shard/InternalIndexingStats.java

ankikuma · 2025-04-29T13:58:46Z

@fcofdez I added a test to IndexingMemoryControllerIT to verify that the stats for total execution time (which includes the time taken to write indexing buffers) are collected and are indeed greater than just the indexing time alone. Please take a look when you get a chance.

fcofdez

Thanks for addressing the feedback, there're a couple of small things to tackle before we merge this 👍 .

fcofdez · 2025-04-30T09:49:23Z

.idea/eclipseCodeFormatter.xml

@@ -1,14 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>


I think that this deletion was accidental? can we revert these changes?

I had deleted the .idea directory from my elasticsearch repo to fix IntelliJ suddenly not recognizing any Lucene libraries, this is an effect of that. will fix it.

server/src/main/java/org/elasticsearch/index/shard/InternalIndexingStats.java

fcofdez · 2025-04-30T09:52:02Z

server/src/internalClusterTest/java/org/elasticsearch/indices/IndexingMemoryControllerIT.java

+        IndexService indexService = createIndex("index", indexSettings(1, 0).put("index.refresh_interval", -1).build());
+        IndexShard shard = indexService.getShard(0);
+        for (int i = 0; i < 100; i++) {
+            prepareIndex("index").setId(Integer.toString(i)).setSource("field", "value").get();


nit: we don't need to set the id explicitly or at least we should add ids randomly, since org.elasticsearch.index.engine.InternalEngine#writeIndexingBuffer takes into account the version map size too (which is populated when explicit ids are used).

Oh I see. Thanks!

fcofdez · 2025-04-30T09:58:23Z

server/src/internalClusterTest/java/org/elasticsearch/indices/IndexingMemoryControllerIT.java

+    public void testIndexingUpdatesRelevantStats() throws Exception {
+        IndexService indexService = createIndex("index", indexSettings(1, 0).put("index.refresh_interval", -1).build());
+        IndexShard shard = indexService.getShard(0);
+        for (int i = 0; i < 100; i++) {


nit: maybe we can do this in a single bulk request?

Actually we don't even need to index so many documents. The stat goes up in every request.

server/src/internalClusterTest/java/org/elasticsearch/indices/IndexingMemoryControllerIT.java

ankikuma · 2025-05-01T01:07:55Z

@fcofdez Should I mark this for backport ?

fcofdez

LGTM. There's a pending comment about a method naming suggestion, but you can merge the PR once that's addressed. Thanks for all the iterations on this 👍

fcofdez · 2025-05-01T12:34:47Z

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

+     * @param took  time it took to write the index buffers for this shard
+     * @see org.elasticsearch.indices.IndexingMemoryController
+     */
+    public void writeIndexBuffersOnIndexThreads(long took) {


can we rename this method to addWriteIndexBuffersOnIndexThreadsTime? its current naming is a bit confusing as it implies that it would do the write. Also, can we include the unit in the parameter method? i.e. tookInNanos?

ankikuma added 2 commits April 14, 2025 11:39

commit

020ee1d

Merge remote-tracking branch 'upstream/main' into 04142025/IWLES11356

4d57998

Refresh to latest

elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 labels Apr 14, 2025

Commit

78f521f

ankikuma added the >enhancement label Apr 15, 2025

Add comments

331595c

ankikuma added the Team:Distributed Indexing Meta label for Distributed Indexing team label Apr 16, 2025

elasticsearchmachine removed the Team:Distributed Indexing Meta label for Distributed Indexing team label Apr 16, 2025

ankikuma requested review from PeteGillinElastic, fcofdez and henningandersen and removed request for PeteGillinElastic and fcofdez April 17, 2025 00:04

elasticsearchmachine and others added 5 commits April 17, 2025 00:10

[CI] Auto commit changes from spotless

7f29b65

commit

d28b2af

Add transport version

7636b32

Merge remote-tracking branch 'upstream/main' into 04142025/IWLES11356

64216a6

Refresh branch

Merge branch '04142025/IWLES11356' of github.com:ankikuma/elasticsear…

6541205

…ch into 04142025/IWLES11356 pull

ankikuma added the :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. label Apr 18, 2025

elasticsearchmachine added Team:Distributed Indexing Meta label for Distributed Indexing team and removed needs:triage Requires assignment of a team area label labels Apr 18, 2025

Update docs/changelog/126786.yaml

0ca6e2d

ankikuma added 2 commits April 22, 2025 13:56

refresh

2f4f8ad

Merge branch '04142025/IWLES11356' of github.com:ankikuma/elasticsear…

018a86c

…ch into 04142025/IWLES11356 commit pull

fcofdez reviewed Apr 22, 2025

View reviewed changes

ankikuma added 4 commits April 25, 2025 10:34

review comments

6dc1af4

review comments

6847ead

Add test

f063913

refresh branch

c24412a

ankikuma requested a review from a team as a code owner April 28, 2025 23:34

name changes

c1c2525

fcofdez reviewed Apr 30, 2025

View reviewed changes

ankikuma added 5 commits April 30, 2025 19:25

refresh

c96bfa4

restore idea files

06d5aa5

address comments

0286b82

simplify test

85685ba

add comments

9061dde

[CI] Auto commit changes from spotless

1d3ba53

fcofdez approved these changes May 1, 2025

View reviewed changes

ankikuma added 7 commits May 1, 2025 10:17

change API name

5df4b76

change API name

e9c944d

refresh

dddc773

more comments

519d907

pull

9541ee5

pull

f00918a

conflict

5b81d57

ankikuma merged commit 084542a into elastic:main May 1, 2025
17 checks passed

Account for time taken to write index buffers in IndexingMemoryController #126786

Account for time taken to write index buffers in IndexingMemoryController #126786

Uh oh!

Conversation

ankikuma commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Apr 18, 2025

Uh oh!

elasticsearchmachine commented Apr 18, 2025

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ankikuma commented Apr 29, 2025

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ankikuma commented May 1, 2025

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ankikuma commented Apr 14, 2025 •

edited

Loading