Skip to content

Commit 71bcbde

Browse files
pugnascotiapugnascotia
committed
Address review feedback
1 parent 6bf3e9b commit 71bcbde

File tree

4 files changed

+18
-72
lines changed

4 files changed

+18
-72
lines changed

‎docs/reference/modules/gateway.asciidoc‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,9 @@ NOTE: These settings only take effect on a full cluster restart.
5252
[[modules-gateway-dangling-indices]]
5353
=== Dangling indices
5454

55-
When a node joins the cluster, it will search for any shards that are
56-
stored in its local data directory and do not already exist in the
57-
cluster. If the static setting `gateway.auto_import_dangling_indices` is
58-
`true` (the default is `false`), then those shards will be imported into
59-
the cluster. This functionality is intended as a best effort to help users
60-
who lose all master nodes. If a new master node is started which is unaware
61-
of the other indices in the cluster, adding the old nodes will cause the
62-
old indices to be imported, instead of being deleted.
63-
64-
Enabling `gateway.auto_import_dangling_indices` should only be done if
65-
absolutely necessary, after understanding the possible consequences (this is not an exhaustive list):
66-
67-
* A deleted index might suddenly reappear when a node joins the cluster.
68-
* You might delete an index and see the immediate creation of another index
69-
with the same name, containing stale mappings and old data.
70-
* New documents could be written to the index before anyone realises that
71-
it has been recovered
72-
* {es} may not be able to find copies of all of the shards of the index,
73-
resulting in a red cluster state.
55+
When a node joins the cluster, any shards stored in its local data
56+
directory which do not already exist in the cluster will be imported into the
57+
cluster. This functionality is intended as a best effort to help users who
58+
lose all master nodes. If a new master node is started which is unaware of
59+
the other indices in the cluster, adding the old nodes will cause the old
60+
indices to be imported, instead of being deleted.

‎server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class DanglingIndicesState implements ClusterStateListener {
5858

5959
public static final Setting<Boolean> AUTO_IMPORT_DANGLING_INDICES_SETTING = Setting.boolSetting(
6060
"gateway.auto_import_dangling_indices",
61-
false,
61+
true,
6262
Setting.Property.NodeScope
6363
);
6464

@@ -76,9 +76,11 @@ public DanglingIndicesState(NodeEnvironment nodeEnv, MetaStateService metaStateS
7676
this.nodeEnv = nodeEnv;
7777
this.metaStateService = metaStateService;
7878
this.allocateDangledIndices = allocateDangledIndices;
79-
clusterService.addListener(this);
80-
8179
this.allocateDanglingIndices = AUTO_IMPORT_DANGLING_INDICES_SETTING.get(clusterService.getSettings());
80+
81+
if (this.allocateDanglingIndices) {
82+
clusterService.addListener(this);
83+
}
8284
}
8385

8486
public void setAllocateDanglingIndicesSetting(boolean allocateDanglingIndices) {

‎server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapAndDetachCommandIT.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import java.util.Locale;
4545

4646
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
47-
import static org.elasticsearch.gateway.DanglingIndicesState.AUTO_IMPORT_DANGLING_INDICES_SETTING;
4847
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
4948
import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING;
5049
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
@@ -397,13 +396,8 @@ public boolean clearData(String nodeName) {
397396
}
398397
});
399398

400-
final Settings settingsWithAutoImport = Settings.builder()
401-
.put(dataNodeDataPathSettings)
402-
.put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), true)
403-
.build();
404-
405399
logger.info("--> start data-only only node and ensure 2 nodes stable cluster");
406-
internalCluster().startDataOnlyNode(settingsWithAutoImport);
400+
internalCluster().startDataOnlyNode(dataNodeDataPathSettings);
407401
ensureStableCluster(2);
408402

409403
logger.info("--> verify that the dangling index exists and has green status");

‎server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java‎

Lines changed: 6 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,9 @@ public void testCleanupWhenEmpty() throws Exception {
5757
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
5858
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
5959

60-
// Given an empty state
6160
assertTrue(danglingState.getDanglingIndices().isEmpty());
62-
63-
// When passed an empty metadata
6461
MetaData metaData = MetaData.builder().build();
6562
danglingState.cleanupAllocatedDangledIndices(metaData);
66-
67-
// Then the state remains empty
6863
assertTrue(danglingState.getDanglingIndices().isEmpty());
6964
}
7065
}
@@ -73,36 +68,24 @@ public void testDanglingIndicesDiscovery() throws Exception {
7368
try (NodeEnvironment env = newNodeEnvironment()) {
7469
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
7570
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
76-
77-
// Given an empty state
7871
assertTrue(danglingState.getDanglingIndices().isEmpty());
79-
80-
// When passed a metdata with an unknown index
8172
MetaData metaData = MetaData.builder().build();
8273
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
8374
IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
8475
metaStateService.writeIndex("test_write", dangledIndex);
8576
Map<Index, IndexMetaData> newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
86-
87-
// Then that index is considered dangling
8877
assertTrue(newDanglingIndices.containsKey(dangledIndex.getIndex()));
89-
90-
// And when passed another metadata with that index
9178
metaData = MetaData.builder().put(dangledIndex, false).build();
9279
newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
93-
94-
// Then then index is not considered to be a new dangling index for a second time
9580
assertFalse(newDanglingIndices.containsKey(dangledIndex.getIndex()));
9681
}
9782
}
9883

9984
public void testInvalidIndexFolder() throws Exception {
10085
try (NodeEnvironment env = newNodeEnvironment()) {
101-
// Given an empty state
10286
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
10387
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
10488

105-
// When passed settings for an index whose folder does not exist
10689
MetaData metaData = MetaData.builder().build();
10790
final String uuid = "test1UUID";
10891
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, uuid);
@@ -113,8 +96,6 @@ public void testInvalidIndexFolder() throws Exception {
11396
Files.move(path, path.resolveSibling("invalidUUID"), StandardCopyOption.ATOMIC_MOVE);
11497
}
11598
}
116-
117-
// Then an exception is thrown describing the problem
11899
try {
119100
danglingState.findNewDanglingIndices(metaData);
120101
fail("no exception thrown for invalid folder name");
@@ -170,31 +151,24 @@ public void testDanglingProcessing() throws Exception {
170151

171152
public void testDanglingIndicesNotImportedWhenTombstonePresent() throws Exception {
172153
try (NodeEnvironment env = newNodeEnvironment()) {
173-
// Given an empty state
174154
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
175155
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
176156

177-
// When passed a dangling index
178157
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
179158
IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
180159
metaStateService.writeIndex("test_write", dangledIndex);
181160

182-
// And there is a tombstone for that index
183161
final IndexGraveyard graveyard = IndexGraveyard.builder().addTombstone(dangledIndex.getIndex()).build();
184162
final MetaData metaData = MetaData.builder().indexGraveyard(graveyard).build();
185-
186-
// Then that index is not imported
187163
assertThat(danglingState.findNewDanglingIndices(metaData).size(), equalTo(0));
188164
}
189165
}
190166

191167
public void testDanglingIndicesStripAliases() throws Exception {
192168
try (NodeEnvironment env = newNodeEnvironment()) {
193-
// Given an empty state
194169
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
195170
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
196171

197-
// When passed an index that has an alias
198172
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
199173
IndexMetaData dangledIndex = IndexMetaData.builder("test1")
200174
.settings(settings)
@@ -205,13 +179,9 @@ public void testDanglingIndicesStripAliases() throws Exception {
205179

206180
final MetaData metaData = MetaData.builder().build();
207181
Map<Index, IndexMetaData> newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
208-
209-
// Then the index is identifying as dangling
210182
assertThat(newDanglingIndices.size(), equalTo(1));
211183
Map.Entry<Index, IndexMetaData> entry = newDanglingIndices.entrySet().iterator().next();
212184
assertThat(entry.getKey().getName(), equalTo("test1"));
213-
214-
// And the alias is removed
215185
assertThat(entry.getValue().getAliases().size(), equalTo(0));
216186
}
217187
}
@@ -220,11 +190,10 @@ public void testDanglingIndicesAreNotAllocatedWhenDisabled() throws Exception {
220190
try (NodeEnvironment env = newNodeEnvironment()) {
221191
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
222192
LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
223-
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices);
193+
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices, false);
224194

225195
assertTrue(danglingState.getDanglingIndices().isEmpty());
226196

227-
// Given a metadata that does not enable allocation of dangling indices
228197
MetaData metaData = MetaData.builder().build();
229198

230199
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
@@ -233,10 +202,8 @@ public void testDanglingIndicesAreNotAllocatedWhenDisabled() throws Exception {
233202

234203
danglingState.findNewAndAddDanglingIndices(metaData);
235204

236-
// When calling the allocate method
237205
danglingState.allocateDanglingIndices();
238206

239-
// Then allocation is not attempted
240207
verify(localAllocateDangledIndices, never()).allocateDangled(any(), any());
241208
}
242209
}
@@ -245,13 +212,10 @@ public void testDanglingIndicesAreAllocatedWhenEnabled() throws Exception {
245212
try (NodeEnvironment env = newNodeEnvironment()) {
246213
MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
247214
LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
248-
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices);
215+
DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices, true);
249216

250217
assertTrue(danglingState.getDanglingIndices().isEmpty());
251218

252-
// Given a state where automatic allocation is enabled
253-
danglingState.setAllocateDanglingIndicesSetting(true);
254-
255219
MetaData metaData = MetaData.builder().build();
256220

257221
final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
@@ -260,24 +224,23 @@ public void testDanglingIndicesAreAllocatedWhenEnabled() throws Exception {
260224

261225
danglingState.findNewAndAddDanglingIndices(metaData);
262226

263-
// When calling the allocate method
264227
danglingState.allocateDanglingIndices();
265228

266-
// Ensure that allocation is attempted
267229
verify(localAllocateDangledIndices).allocateDangled(any(), any());
268230
}
269231
}
270232

271233
private DanglingIndicesState createDanglingIndicesState(NodeEnvironment env, MetaStateService metaStateService) {
272-
return createDanglingIndicesState(env, metaStateService, null);
234+
return createDanglingIndicesState(env, metaStateService, null, true);
273235
}
274236

275237
private DanglingIndicesState createDanglingIndicesState(
276238
NodeEnvironment env,
277239
MetaStateService metaStateService,
278-
LocalAllocateDangledIndices indexAllocator
240+
LocalAllocateDangledIndices indexAllocator,
241+
boolean shouldAutoImport
279242
) {
280-
final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), false).build();
243+
final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), shouldAutoImport).build();
281244

282245
final ClusterService clusterServiceMock = mock(ClusterService.class);
283246
when(clusterServiceMock.getClusterSettings()).thenReturn(

0 commit comments

Comments
 (0)