Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return cached segments stats if include_unloaded_segments is true #39698

Merged
merged 5 commits into from
Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@
import org.apache.lucene.index.IndexCommit;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.LeafReader;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.SegmentReader;
import org.apache.lucene.store.Directory;
import org.elasticsearch.common.lucene.Lucene;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.List;
import java.util.function.Function;

Expand All @@ -36,8 +40,20 @@
*/
public final class NoOpEngine extends ReadOnlyEngine {

private final SegmentsStats stats;

public NoOpEngine(EngineConfig config) {
super(config, null, null, true, Function.identity());
this.stats = new SegmentsStats();
Directory directory = store.directory();
try (DirectoryReader reader = DirectoryReader.open(directory)) {
for (LeafReaderContext ctx : reader.getContext().leaves()) {
SegmentReader segmentReader = Lucene.segmentReader(ctx.reader());
fillSegmentStats(segmentReader, true, stats);
}
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

@Override
Expand All @@ -47,17 +63,17 @@ protected DirectoryReader open(final IndexCommit commit) throws IOException {
final IndexCommit indexCommit = indexCommits.get(indexCommits.size() - 1);
return new DirectoryReader(directory, new LeafReader[0]) {
@Override
protected DirectoryReader doOpenIfChanged() throws IOException {
protected DirectoryReader doOpenIfChanged() {
return null;
}

@Override
protected DirectoryReader doOpenIfChanged(IndexCommit commit) throws IOException {
protected DirectoryReader doOpenIfChanged(IndexCommit commit) {
return null;
}

@Override
protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws IOException {
protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) {
return null;
}

Expand All @@ -67,17 +83,17 @@ public long getVersion() {
}

@Override
public boolean isCurrent() throws IOException {
public boolean isCurrent() {
return true;
}

@Override
public IndexCommit getIndexCommit() throws IOException {
public IndexCommit getIndexCommit() {
return indexCommit;
}

@Override
protected void doClose() throws IOException {
protected void doClose() {
}

@Override
Expand All @@ -86,4 +102,18 @@ public CacheHelper getReaderCacheHelper() {
}
};
}

@Override
public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean includeUnloadedSegments) {
if (includeUnloadedSegments) {
final SegmentsStats stats = new SegmentsStats();
stats.add(this.stats);
if (includeSegmentFileSizes == false) {
stats.clearFileSizes();
}
return stats;
} else {
return super.segmentsStats(includeSegmentFileSizes, includeUnloadedSegments);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling this will fail on a NoopEngine?
Should we instead return an empty stats object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm there is a test for this here that shows that it's not failing but returning an empty object.

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void testNoopAfterRegularEngine() throws IOException {
noOpEngine.close();
}

public void testNoOpEngineDocStats() throws Exception {
public void testNoOpEngineStats() throws Exception {
IOUtils.close(engine, store);
final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED);
try (Store store = createStore()) {
Expand Down Expand Up @@ -131,15 +131,25 @@ public void testNoOpEngineDocStats() throws Exception {
}

final DocsStats expectedDocStats;
boolean includeFileSize = randomBoolean();
final SegmentsStats expectedSegmentStats;
try (InternalEngine engine = createEngine(config)) {
expectedDocStats = engine.docStats();
expectedSegmentStats = engine.segmentsStats(includeFileSize, true);
}

try (NoOpEngine noOpEngine = new NoOpEngine(config)) {
assertEquals(expectedDocStats.getCount(), noOpEngine.docStats().getCount());
assertEquals(expectedDocStats.getDeleted(), noOpEngine.docStats().getDeleted());
assertEquals(expectedDocStats.getTotalSizeInBytes(), noOpEngine.docStats().getTotalSizeInBytes());
assertEquals(expectedDocStats.getAverageSizeInBytes(), noOpEngine.docStats().getAverageSizeInBytes());
assertEquals(expectedSegmentStats.getCount(), noOpEngine.segmentsStats(includeFileSize, true).getCount());
assertEquals(expectedSegmentStats.getMemoryInBytes(), noOpEngine.segmentsStats(includeFileSize, true).getMemoryInBytes());
assertEquals(expectedSegmentStats.getFileSizes().size(),
noOpEngine.segmentsStats(includeFileSize, true).getFileSizes().size());

assertEquals(0, noOpEngine.segmentsStats(includeFileSize, false).getFileSizes().size());
assertEquals(0, noOpEngine.segmentsStats(includeFileSize, false).getMemoryInBytes());
} catch (AssertionError e) {
logger.error(config.getMergePolicy());
throw e;
Expand Down