Writable warm replica replication/recovery #17390
Conversation
Signed-off-by: Sandeep Kumawat <[email protected]>
github-actions
bot
added
enhancement
skumawat2025
changed the title
|
❌ Gradle check result for 6fde641: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
| @@ -368,6 +368,11 @@ public boolean shouldPeriodicallyFlush() { | |||
| @Override | |||
| public void flush(boolean force, boolean waitIfOngoing) throws EngineException { | |||
| ensureOpen(); | |||
| // Skip flushing for indices with partial locality (warm indices) | |||
| // For these indices, we don't need to commit as we will sync from the remote store on re-open | |||
| if (engineConfig.getIndexSettings().isStoreLocalityPartial() == false) { | |||
There was a problem hiding this comment.
shouldn't be this engineConfig.getIndexSettings().isStoreLocalityPartial() ?
There was a problem hiding this comment.
Correct. This is the right check. Thanks
| @@ -238,7 +238,8 @@ private IndexMetadata.Builder updateInSyncAllocations( | |||
| allocationId = RecoverySource.ExistingStoreRecoverySource.FORCED_ALLOCATION_ID; | |||
| } else { | |||
| assert (recoverySource instanceof RecoverySource.SnapshotRecoverySource | |||
| || recoverySource instanceof RecoverySource.RemoteStoreRecoverySource) : recoverySource; | |||
| || recoverySource instanceof RecoverySource.RemoteStoreRecoverySource | |||
| || recoverySource instanceof RecoverySource.ExistingStoreRecoverySource) : recoverySource; | |||
There was a problem hiding this comment.
why is this required and what will be the implication ?
| // Return an empty list for warm indices, In this case, replica shards don't require downloading files from remote storage | ||
| // as they can will sync all files from remote in case failure. |
There was a problem hiding this comment.
should we even the replicas checkpoint in case of warm shards ? Also make sure to handle the replication lag metric .
| @@ -190,8 +190,10 @@ public CacheStats stats() { | |||
| public void logCurrentState() { | |||
| int i = 0; | |||
| for (RefCountedCache<K, V> cache : table) { | |||
| logger.trace("SegmentedCache " + i); | |||
| ((LRUCache<K, V>) cache).logCurrentState(); | |||
| if (cache.size() > 0) { | |||
There was a problem hiding this comment.
can we handle size() == 0 case inside logCurrentState ?
| logger.trace("Composite Directory[{}]: Local Directory files - {}", this::toString, () -> Arrays.toString(localFiles)); | ||
| logger.trace("Composite Directory[{}]: Remote Directory files - {}", this::toString, () -> Arrays.toString(remoteFiles)); | ||
| // logger.trace("Composite Directory[{}]: Remote Directory files - {}", this::toString, () -> Arrays.toString(remoteFiles)); | ||
| Set<String> nonBlockLuceneFiles = allFiles.stream() |
There was a problem hiding this comment.
why are we returning just the nonBlockLuceneFiles here ? won't the list be incomplete here as some files can get evicted ?
Description
This PR aims to implement the Replica Replication and Recovery flow for indices with partial store locality (also known as writable warm indices), which were introduced with the composite directory in PR #12782. It builds upon and addresses comments received on the pending PR #14670.
For writable indices, where primary shards now consist of both localDirectory and remoteDirectory, we've implemented file uploads to remote storage and maintain an LRU fileCache on the node instead of storing all files locally.
We've made several changes to the replication and recovery processes:
Related Issues
Resolves #13647
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.