Skip to content

Conversation

@masseyke
Copy link
Member

There are cases where we need access to some computationally-expensive resource in TransportBroadcastByNodeAction::shardOperation. Even if that resource is not dependent on the particular shard in that method, we currently have to recompute it because there is no way to share resources across calls to shardOperation. For example, when we call GET _stats, we have to scan through all shards in the cache to compute the shared memory in the cache -- and we do this same computation for each shard (see #97222).
This PR introduces the notion of a NodeContext to a TransportBroadcastByNodeAction. The default implementation does nothing, but a NodeContext can be created by the createNodeContext method, which is called one time per node before the calls to shardOperation begin.
This PR does not change any current functionality, but future PRs will take advantage of it.

@masseyke masseyke added >non-issue :Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. v9.3.0 labels Nov 13, 2025
@joegallo
Copy link
Contributor

I think it's conventional to have the listener be the last argument, but I'm just saying that as a passer-by and I'll leave it up to @DaveCTurner to say whether that's something real or imagined.

@masseyke
Copy link
Member Author

I think it's conventional to have the listener be the last argument, but I'm just saying that as a passer-by and I'll leave it up to @DaveCTurner to say whether that's something real or imagined.

Done!

@masseyke masseyke marked this pull request as ready for review November 13, 2025 22:22
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Nov 13, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@DaveCTurner
Copy link
Contributor

I think it's conventional to have the listener be the last argument

Yes it is, thanks for spotting/fixing that

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.

LGTM (left a few nits but nothing needing another look)

@masseyke masseyke merged commit 9df1314 into elastic:main Nov 14, 2025
34 checks passed
@masseyke masseyke deleted the add-nodeContext-to-TransportBroadcastByNodeAction branch November 14, 2025 20:31
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Nov 16, 2025
* main: (135 commits)
  Mute org.elasticsearch.upgrades.IndexSortUpgradeIT testIndexSortForNumericTypes {upgradedNodes=1} elastic#138130
  Mute org.elasticsearch.upgrades.IndexSortUpgradeIT testIndexSortForNumericTypes {upgradedNodes=2} elastic#138129
  Mute org.elasticsearch.search.basic.SearchWithRandomDisconnectsIT testSearchWithRandomDisconnects elastic#138128
  [DiskBBQ] avoid EsAcceptDocs bug by calling cost before building iterator (elastic#138127)
  Log NOT_PREFERRED shard movements (elastic#138069)
  Improve bulk loading of binary doc values (elastic#137995)
  Add internal action for getting inference fields and inference results for those fields (elastic#137680)
  Address issue with DateFieldMapper#isFieldWithinQuery(...) (elastic#138032)
  WriteLoadConstraintDecider: Have separate rate limiting for canRemain and canAllocate decisions (elastic#138067)
  Adding NodeContext to TransportBroadcastByNodeAction (elastic#138057)
  Mute org.elasticsearch.simdvec.ESVectorUtilTests testSoarDistanceBulk elastic#138117
  Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT test elastic#137909
  Backport batched_response_might_include_reduction_failure version to 8.19 (elastic#138046)
  Add summary metrics for tdigest fields (elastic#137982)
  Add gp-llm-v2 model ID and inference endpoint (elastic#138045)
  Various tracing fixes (elastic#137908)
  [ML] Fixing KDE evaluate() to return correct ValueAndMagnitude object (elastic#128602)
  Mute org.elasticsearch.xpack.shutdown.NodeShutdownIT testStalledShardMigrationProperlyDetected elastic#115697
  [ML] Fix Flaky Audit Message Assertion in testWithDatastream for RegressionIT and ClassificationIT (elastic#138065)
  [ML] Fix Non-Deterministic Training Set Selection in RegressionIT testTwoJobsWithSameRandomizeSeedUseSameTrainingSet (elastic#138063)
  ...

# Conflicts:
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.vectors/200_dense_vector_docvalue_fields.yml
masseyke added a commit to masseyke/elasticsearch that referenced this pull request Nov 26, 2025
masseyke added a commit to masseyke/elasticsearch that referenced this pull request Nov 26, 2025
@masseyke
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
9.2
9.1
8.19

Questions ?

Please refer to the Backport tool documentation

masseyke added a commit to masseyke/elasticsearch that referenced this pull request Nov 26, 2025
(cherry picked from commit 9df1314)

# Conflicts:
#	server/src/main/java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeAction.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport pending :Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v8.19.9 v9.1.9 v9.2.3 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants