Repository navigation
Do not abort shard bulk on expansion rejection - #158235
Conversation
An indexing pressure rejection could be thrown from the middle of the primary bulk item loop, since setRequestToExecute tracked expanded bytes for every item. The exception escaped performOnPrimary and aborted the whole BulkShardRequest before replication, so operations already applied to the primary were never replicated, permanently stranding the replica local checkpoint below max_seq_no and pinning the global checkpoint. Track the expansion only for translated updates, within the item try/catch, so a rejection fails just that update and the remaining operations execute and replicate normally. Closes elastic#158212
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @fcofdez, I've created a changelog YAML for you. |
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
…:fcofdez/elasticsearch into fix-mid-bulk-indexing-pressure-rejection
…-pressure-rejection
| private void assertLocalCheckpointsReachMaxSeqNo() throws Exception { | ||
| assertBusy(() -> { | ||
| for (ShardStats shardStats : indicesAdmin().prepareStats(INDEX_NAME).get().getShards()) { | ||
| final SeqNoStats seqNoStats = shardStats.getSeqNoStats(); |
There was a problem hiding this comment.
Just as a double check, can we also verify that the max is the same across the copies?
Can be a follow-up.
tlrx
left a comment
There was a problem hiding this comment.
LGTM. I wonder if ShardBatchIndexer suffered from the same bug and if so, if it deserves its own test in a follow-up.
| } | ||
| assertEquals(1L, primaryPressure.stats().getPrimaryRejections()); | ||
|
|
||
| assertLocalCheckpointsReachMaxSeqNo(); |
There was a problem hiding this comment.
Possible follow-up, could we also ESIntegTestCase#assertSameDocIdsOnShards?
AFAIK, the batch mode does not support updates and fallbacks into the sequential mode . With the changes in the PR we should be good on that front. |
|
Thanks for the reviews! |
💔 Backport failed
You can use sqren/backport to manually backport by running |
An indexing pressure rejection could be thrown from the middle of the primary bulk item loop, since setRequestToExecute tracked expanded bytes for every item. The exception escaped performOnPrimary and aborted the whole BulkShardRequest before replication, so operations already applied to the primary were never replicated, permanently stranding the replica local checkpoint below max_seq_no and pinning the global checkpoint. Track the expansion only for translated updates, within the item try/catch, so a rejection fails just that update and the remaining operations execute and replicate normally. Closes #158212 Backport of #158235
An indexing pressure rejection could be thrown from the middle of
the primary bulk item loop, since setRequestToExecute tracked
expanded bytes for every item. The exception escaped
performOnPrimary and aborted the whole BulkShardRequest before
replication, so operations already applied to the primary were
never replicated, permanently stranding the replica local
checkpoint below max_seq_no and pinning the global checkpoint.
Track the expansion only for translated updates, within the item
try/catch, so a rejection fails just that update and the remaining
operations execute and replicate normally.
Closes #158212