Skip to content

Commit 2a2b613

Browse files
authored
TransportVerifyShardBeforeCloseAction should force a flush (#38401)
This commit changes the `TransportVerifyShardBeforeCloseAction` so that it always forces the flush of the shard. It seems that #37961 is not sufficient to ensure that the translog and the Lucene commit share the exact same max seq no and global checkpoint information in case of one or more noop operations have been made. The `BulkWithUpdatesIT.testThatMissingIndexDoesNotAbortFullBulkRequest` and `FrozenIndexTests.testFreezeEmptyIndexWithTranslogOps` test this trivial situation and they both fail 1 on 10 executions. Relates to #33888
1 parent a0a2479 commit 2a2b613

File tree

5 files changed

+22
-16
lines changed

5 files changed

+22
-16
lines changed

plugins/repository-hdfs/src/test/resources/rest-api-spec/test/hdfs_repository/40_restore.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@
6262

6363
- match: { test_index.shards.0.type: SNAPSHOT }
6464
- match: { test_index.shards.0.stage: DONE }
65-
- match: { test_index.shards.0.index.files.recovered: 0}
66-
- match: { test_index.shards.0.index.size.recovered_in_bytes: 0}
67-
- match: { test_index.shards.0.index.files.reused: 1}
68-
- gt: { test_index.shards.0.index.size.reused_in_bytes: 0}
65+
- match: { test_index.shards.0.index.files.recovered: 1}
66+
- gt: { test_index.shards.0.index.size.recovered_in_bytes: 0}
67+
- match: { test_index.shards.0.index.files.reused: 0}
68+
- match: { test_index.shards.0.index.size.reused_in_bytes: 0}
6969

7070
# Remove our snapshot
7171
- do:

plugins/repository-hdfs/src/test/resources/rest-api-spec/test/secure_hdfs_repository/40_restore.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@
6464

6565
- match: { test_index.shards.0.type: SNAPSHOT }
6666
- match: { test_index.shards.0.stage: DONE }
67-
- match: { test_index.shards.0.index.files.recovered: 0}
68-
- match: { test_index.shards.0.index.size.recovered_in_bytes: 0}
69-
- match: { test_index.shards.0.index.files.reused: 1}
70-
- gt: { test_index.shards.0.index.size.reused_in_bytes: 0}
67+
- match: { test_index.shards.0.index.files.recovered: 1}
68+
- gt: { test_index.shards.0.index.size.recovered_in_bytes: 0}
69+
- match: { test_index.shards.0.index.files.reused: 0}
70+
- match: { test_index.shards.0.index.size.reused_in_bytes: 0}
7171

7272
# Remove our snapshot
7373
- do:

rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.restore/10_basic.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ setup:
5353

5454
- match: { test_index.shards.0.type: SNAPSHOT }
5555
- match: { test_index.shards.0.stage: DONE }
56-
- match: { test_index.shards.0.index.files.recovered: 0}
57-
- match: { test_index.shards.0.index.size.recovered_in_bytes: 0}
58-
- match: { test_index.shards.0.index.files.reused: 1}
59-
- gt: { test_index.shards.0.index.size.reused_in_bytes: 0}
56+
- gte: { test_index.shards.0.index.files.recovered: 0}
57+
- gte: { test_index.shards.0.index.size.recovered_in_bytes: 0}
58+
- gte: { test_index.shards.0.index.files.reused: 0}
59+
- gte: { test_index.shards.0.index.size.reused_in_bytes: 0}

server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportVerifyShardBeforeCloseAction.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,8 @@ private void executeShardOperation(final ShardRequest request, final IndexShard
115115
+ "] mismatches maximum sequence number [" + maxSeqNo + "] on index shard " + shardId);
116116
}
117117

118-
final boolean forced = indexShard.isSyncNeeded();
119-
indexShard.flush(new FlushRequest().force(forced));
120-
logger.trace("{} shard is ready for closing [forced:{}]", shardId, forced);
118+
indexShard.flush(new FlushRequest().force(true));
119+
logger.trace("{} shard is ready for closing", shardId);
121120
}
122121

123122
@Override

server/src/test/java/org/elasticsearch/action/admin/indices/close/TransportVerifyShardBeforeCloseActionTests.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.elasticsearch.cluster.routing.ShardRoutingState;
4040
import org.elasticsearch.cluster.service.ClusterService;
4141
import org.elasticsearch.common.settings.Settings;
42+
import org.elasticsearch.index.engine.Engine;
4243
import org.elasticsearch.index.seqno.SeqNoStats;
4344
import org.elasticsearch.index.seqno.SequenceNumbers;
4445
import org.elasticsearch.index.shard.IndexShard;
@@ -56,6 +57,7 @@
5657
import org.junit.AfterClass;
5758
import org.junit.Before;
5859
import org.junit.BeforeClass;
60+
import org.mockito.ArgumentCaptor;
5961

6062
import java.util.Collections;
6163
import java.util.List;
@@ -69,6 +71,7 @@
6971
import static org.hamcrest.Matchers.equalTo;
7072
import static org.hamcrest.Matchers.greaterThan;
7173
import static org.hamcrest.Matchers.instanceOf;
74+
import static org.hamcrest.Matchers.is;
7275
import static org.mockito.Matchers.any;
7376
import static org.mockito.Mockito.mock;
7477
import static org.mockito.Mockito.times;
@@ -144,9 +147,13 @@ private void executeOnPrimaryOrReplica() throws Exception {
144147
}
145148
}
146149

147-
public void testOperationSuccessful() throws Exception {
150+
public void testShardIsFlushed() throws Exception {
151+
final ArgumentCaptor<FlushRequest> flushRequest = ArgumentCaptor.forClass(FlushRequest.class);
152+
when(indexShard.flush(flushRequest.capture())).thenReturn(new Engine.CommitId(new byte[0]));
153+
148154
executeOnPrimaryOrReplica();
149155
verify(indexShard, times(1)).flush(any(FlushRequest.class));
156+
assertThat(flushRequest.getValue().force(), is(true));
150157
}
151158

152159
public void testOperationFailsWithOnGoingOps() {

0 commit comments

Comments
 (0)