Add plumbing logic to invoke shard indexing pressure during write operation. #497
Conversation
Signed-off-by: Saurabh Singh <sisurab@amazon.com>
…ration. (#478) Signed-off-by: Saurabh Singh <sisurab@amazon.com>
|
✅ DCO Check Passed 8ab960a |
|
✅ Gradle Wrapper Validation success 8ab960a |
getsaurabh02
changed the title
|
✅ Gradle Precommit success 8ab960a |
| return indexingPressure.markReplicaOperationStarted(replicaOperationSize(request), force(request)); | ||
| if (indexingPressure.isShardIndexingPressureEnabled() == false) { | ||
| return indexingPressure.markReplicaOperationStarted(replicaOperationSize(request), force(request)); | ||
| } else { | ||
| return () -> {}; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected Releasable checkShardReplicaLimits(ReplicaRequest request) { | ||
| if (shardIndexingPressure.isShardIndexingPressureEnabled()) { | ||
| return shardIndexingPressure.markReplicaOperationStarted(request.shardId, replicaOperationSize(request), force(request)); | ||
| } else { | ||
| return () -> {}; | ||
| } |
There was a problem hiding this comment.
I guess the abstraction could be simplified. Essentially what we are doing here is putting the knowledge of the back pressure mechanism across the board forcing top layers to know the mechanism when ideally they should be agnostic. As you note here shardIndexingPressure.isShardIndexingPressureEnabled() that both shard indexing pressure and indexing pressure cannot mutually co-exist. I advise we simplify this and abstract this knowledge out in low level components
Also from the comment you noted you intended to keep "ShardIndexingPressure as a sub feature and is contained with IndexingPressure"
I am sorry I don't follow you completely based on this part of the code
There was a problem hiding this comment.
I agree there was some scope of simplification here. I have made the required structural changes by introducing a top level construct for IndexingPressure as IndexingPressureService in the latest update #496. This largely allows the orchestration of the indexing pressure invocations from all the places in the TransportAction, thereby allowing the top layers to be agnostic of the low level implementation and feature-settings. I see with this change the amount of plumbing changes have reduced with mostly API changes, and one additional call in case of coordinating node. This is all covered in #496 now.
Note: We can update/close this PR shortly after #496 is merged as plumbing changes are covered in that now.
Description
This PR addresses 2nd out of the 4 planned Items for Shard Indexing Pressure ((#478). Add plumbing logic to invoke shard indexing pressure during write operation.
Changes to Review : 8ab960a
Currently this PR is inclusive of the 1st change (#496), for build and precommit to succeed. Reviewer's are requested to look only at the last commit changes here, and skip the the file changed in previous commits (#496). Once (#496) is merged, will update the branch to have only the top commit here.
Co-authored-by: Dharmesh Singh sdharms@amazon.com
Issues Resolved
Addresses Item 2 of #478
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.