From 51c9f739476743f7b1fb3a97a90df7330e8e289e Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 20 May 2015 11:33:07 +0200 Subject: [PATCH] Include num_docs in the commit stats This also fixes a potential race condition when the number of docs is compared across shards with the same seal ID since the assertion was taking the number of docs form the live index reader which might not be equivalent to the committed num docs. --- .../org/elasticsearch/common/lucene/Lucene.java | 2 +- .../elasticsearch/index/engine/CommitStats.java | 15 +++++++++++++++ .../index/engine/InternalEngine.java | 1 + .../elasticsearch/test/InternalTestCluster.java | 6 ++++-- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 19b8cdbc08e..e38af307a8a 100644 --- a/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -146,7 +146,7 @@ public class Lucene { } /** - * Returns the number of document in the index referenced by this {@link SegmentInfos} + * Returns the number of documents in the index referenced by this {@link SegmentInfos} */ public static int getNumDocs(SegmentInfos info) { int numDocs = 0; diff --git a/src/main/java/org/elasticsearch/index/engine/CommitStats.java b/src/main/java/org/elasticsearch/index/engine/CommitStats.java index 6e5d26c8e81..ed0a1a6a284 100644 --- a/src/main/java/org/elasticsearch/index/engine/CommitStats.java +++ b/src/main/java/org/elasticsearch/index/engine/CommitStats.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilderString; @@ -37,6 +38,7 @@ public final class CommitStats implements Streamable, ToXContent { private Map userData; private long generation; private String id; // lucene commit id in base 64; + private int numDocs; public CommitStats(SegmentInfos segmentInfos) { // clone the map to protect against concurrent changes @@ -46,6 +48,7 @@ public final class CommitStats implements Streamable, ToXContent { if (segmentInfos.getId() != null) { // id is only written starting with Lucene 5.0 id = Base64.encodeBytes(segmentInfos.getId()); } + numDocs = Lucene.getNumDocs(segmentInfos); } private CommitStats() { @@ -76,6 +79,13 @@ public final class CommitStats implements Streamable, ToXContent { return id; } + /** + * Returns the number of documents in the in this commit + */ + public int getNumDocs() { + return numDocs; + } + @Override public void readFrom(StreamInput in) throws IOException { MapBuilder builder = MapBuilder.newMapBuilder(); @@ -85,6 +95,7 @@ public final class CommitStats implements Streamable, ToXContent { userData = builder.immutableMap(); generation = in.readLong(); id = in.readString(); + numDocs = in.readInt(); } @Override @@ -96,6 +107,7 @@ public final class CommitStats implements Streamable, ToXContent { } out.writeLong(generation); out.writeString(id); + out.writeInt(numDocs); } static final class Fields { @@ -103,6 +115,8 @@ public final class CommitStats implements Streamable, ToXContent { static final XContentBuilderString USER_DATA = new XContentBuilderString("user_data"); static final XContentBuilderString ID = new XContentBuilderString("id"); static final XContentBuilderString COMMIT = new XContentBuilderString("commit"); + static final XContentBuilderString NUM_DOCS = new XContentBuilderString("num_docs"); + } @Override @@ -111,6 +125,7 @@ public final class CommitStats implements Streamable, ToXContent { builder.field(Fields.ID, id); builder.field(Fields.GENERATION, generation); builder.field(Fields.USER_DATA, userData); + builder.field(Fields.NUM_DOCS, numDocs); builder.endObject(); return builder; } diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 8504340857d..151298acd07 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -1224,4 +1224,5 @@ public class InternalEngine extends Engine { private void commitIndexWriter(IndexWriter writer, Translog translog) throws IOException { commitIndexWriter(writer, translog, null); } + } diff --git a/src/test/java/org/elasticsearch/test/InternalTestCluster.java b/src/test/java/org/elasticsearch/test/InternalTestCluster.java index bfdf951ccc6..a12a34e8f46 100644 --- a/src/test/java/org/elasticsearch/test/InternalTestCluster.java +++ b/src/test/java/org/elasticsearch/test/InternalTestCluster.java @@ -74,6 +74,7 @@ import org.elasticsearch.index.cache.filter.FilterCacheModule; import org.elasticsearch.index.cache.filter.FilterCacheModule.FilterCacheSettings; import org.elasticsearch.index.cache.filter.index.IndexFilterCache; import org.elasticsearch.index.cache.filter.none.NoneFilterCache; +import org.elasticsearch.index.engine.CommitStats; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.EngineClosedException; import org.elasticsearch.index.shard.IndexShard; @@ -993,9 +994,10 @@ public final class InternalTestCluster extends TestCluster { for (IndexService indexService : indexServices) { for (IndexShard indexShard : indexService) { try { - String syncId = indexShard.engine().commitStats().getUserData().get(Engine.SYNC_COMMIT_ID); + CommitStats commitStats = indexShard.engine().commitStats(); + String syncId = commitStats.getUserData().get(Engine.SYNC_COMMIT_ID); if (syncId != null) { - long liveDocsOnShard = indexShard.docStats().getCount() - indexShard.docStats().getDeleted(); + long liveDocsOnShard = commitStats.getNumDocs(); if (docsOnShards.get(syncId) != null) { assertThat("sync id is equal but number of docs does not match on node " + nodeAndClient.name + ". expected " + docsOnShards.get(syncId) + " but got " + liveDocsOnShard, docsOnShards.get(syncId), equalTo(liveDocsOnShard)); } else {