Skip to content

Commit 4f06225

Browse files
committed
Avoid unneeded refresh with concurrent realtime gets (#47895)
This change should reduce refreshes for a use-case where we perform multiple realtime gets at the same time on an active index. Currently, we only call refresh if the index operation is still on the versionMap. However, at the time we call refresh, that operation might be already or will be included in the latest reader. Hence, we do not need to refresh. Adding another lock here is not an issue as the refresh is already sequential.
1 parent 4c1bb21 commit 4f06225

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,8 @@ public GetResult get(Get get, BiFunction<String, SearcherScope, Engine.Searcher>
677677
trackTranslogLocation.set(true);
678678
}
679679
}
680-
refresh("realtime_get", SearcherScope.INTERNAL, true);
680+
assert versionValue.seqNo >= 0 : versionValue;
681+
refreshIfNeeded("realtime_get", versionValue.seqNo);
681682
}
682683
scope = SearcherScope.INTERNAL;
683684
} else {

server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

+55
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@
165165
import java.util.concurrent.BrokenBarrierException;
166166
import java.util.concurrent.CountDownLatch;
167167
import java.util.concurrent.CyclicBarrier;
168+
import java.util.concurrent.Phaser;
168169
import java.util.concurrent.Semaphore;
169170
import java.util.concurrent.TimeUnit;
170171
import java.util.concurrent.atomic.AtomicBoolean;
@@ -6094,4 +6095,58 @@ private void runTestDeleteFailure(
60946095
}
60956096
}
60966097

6098+
public void testRealtimeGetOnlyRefreshIfNeeded() throws Exception {
6099+
final AtomicInteger refreshCount = new AtomicInteger();
6100+
final ReferenceManager.RefreshListener refreshListener = new ReferenceManager.RefreshListener() {
6101+
@Override
6102+
public void beforeRefresh() {
6103+
6104+
}
6105+
6106+
@Override
6107+
public void afterRefresh(boolean didRefresh) {
6108+
if (didRefresh) {
6109+
refreshCount.incrementAndGet();
6110+
}
6111+
}
6112+
};
6113+
try (Store store = createStore()) {
6114+
final EngineConfig config = config(defaultSettings, store, createTempDir(), newMergePolicy(), null,
6115+
refreshListener, null, null, engine.config().getCircuitBreakerService());
6116+
try (InternalEngine engine = createEngine(config)) {
6117+
int numDocs = randomIntBetween(10, 100);
6118+
Set<String> ids = new HashSet<>();
6119+
for (int i = 0; i < numDocs; i++) {
6120+
String id = Integer.toString(i);
6121+
engine.index(indexForDoc(createParsedDoc(id, null)));
6122+
ids.add(id);
6123+
}
6124+
final int refreshCountBeforeGet = refreshCount.get();
6125+
Thread[] getters = new Thread[randomIntBetween(1, 4)];
6126+
Phaser phaser = new Phaser(getters.length + 1);
6127+
for (int t = 0; t < getters.length; t++) {
6128+
getters[t] = new Thread(() -> {
6129+
phaser.arriveAndAwaitAdvance();
6130+
int iters = randomIntBetween(1, 10);
6131+
for (int i = 0; i < iters; i++) {
6132+
ParsedDocument doc = createParsedDoc(randomFrom(ids), null);
6133+
try (Engine.GetResult getResult = engine.get(newGet(true, doc), engine::acquireSearcher)) {
6134+
assertThat(getResult.exists(), equalTo(true));
6135+
assertThat(getResult.docIdAndVersion(), notNullValue());
6136+
}
6137+
}
6138+
});
6139+
getters[t].start();
6140+
}
6141+
phaser.arriveAndAwaitAdvance();
6142+
for (int i = 0; i < numDocs; i++) {
6143+
engine.index(indexForDoc(createParsedDoc("more-" + i, null)));
6144+
}
6145+
for (Thread getter : getters) {
6146+
getter.join();
6147+
}
6148+
assertThat(refreshCount.get(), lessThanOrEqualTo(refreshCountBeforeGet + 1));
6149+
}
6150+
}
6151+
}
60976152
}

0 commit comments

Comments
 (0)