Skip to content

Commit cd82da5

Browse files
parkertimminsparkertimmins
authored
Improve how reindex data stream index action handles api blocks (#120084)
- fail quickly if index has read or metadata block - add write block, but proceed if already has read-only or read-only-allow-delete - filter write, read-only, and read-only-allow-delete from dest index - results in dest index having no blocks
1 parent 825b5de commit cd82da5

File tree

3 files changed

+80
-25
lines changed

3 files changed

+80
-25
lines changed

‎docs/changelog/120084.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 120084
2+
summary: Improve how reindex data stream index action handles api blocks
3+
area: Data streams
4+
type: enhancement
5+
issues: []

‎x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java‎

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.migrate.action;
99

10+
import org.elasticsearch.ElasticsearchException;
1011
import org.elasticsearch.action.DocWriteRequest;
1112
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
1213
import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
@@ -224,16 +225,46 @@ public void testMappingsAddedToDestIndex() throws Exception {
224225
assertEquals("text", XContentMapValues.extractValue("properties.foo1.type", destMappings));
225226
}
226227

227-
public void testReadOnlyAddedBack() {
228+
public void testFailIfMetadataBlockSet() {
229+
assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled());
230+
231+
var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT);
232+
var settings = Settings.builder().put(IndexMetadata.SETTING_BLOCKS_METADATA, true).build();
233+
indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet();
234+
235+
try {
236+
client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)).actionGet();
237+
} catch (ElasticsearchException e) {
238+
assertTrue(e.getMessage().contains("Cannot reindex index") || e.getCause().getMessage().equals("Cannot reindex index"));
239+
}
240+
241+
cleanupMetadataBlocks(sourceIndex);
242+
}
243+
244+
public void testFailIfReadBlockSet() {
245+
assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled());
246+
247+
var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT);
248+
var settings = Settings.builder().put(IndexMetadata.SETTING_BLOCKS_READ, true).build();
249+
indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet();
250+
251+
try {
252+
client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)).actionGet();
253+
} catch (ElasticsearchException e) {
254+
assertTrue(e.getMessage().contains("Cannot reindex index") || e.getCause().getMessage().equals("Cannot reindex index"));
255+
}
256+
257+
cleanupMetadataBlocks(sourceIndex);
258+
}
259+
260+
public void testReadOnlyBlocksNotAddedBack() {
228261
assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled());
229262

230-
// Create source index with read-only and/or block-writes
231263
var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT);
232-
boolean isReadOnly = randomBoolean();
233-
boolean isBlockWrites = randomBoolean();
234264
var settings = Settings.builder()
235-
.put(IndexMetadata.SETTING_READ_ONLY, isReadOnly)
236-
.put(IndexMetadata.SETTING_BLOCKS_WRITE, isBlockWrites)
265+
.put(IndexMetadata.SETTING_READ_ONLY, randomBoolean())
266+
.put(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE, randomBoolean())
267+
.put(IndexMetadata.SETTING_BLOCKS_WRITE, randomBoolean())
237268
.build();
238269
indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet();
239270

@@ -242,13 +273,13 @@ public void testReadOnlyAddedBack() {
242273
.actionGet()
243274
.getDestIndex();
244275

245-
// assert read-only settings added back to dest index
246276
var settingsResponse = indicesAdmin().getSettings(new GetSettingsRequest().indices(destIndex)).actionGet();
247-
assertEquals(isReadOnly, Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY)));
248-
assertEquals(isBlockWrites, Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE)));
277+
assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY)));
278+
assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE)));
279+
assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE)));
249280

250-
removeReadOnly(sourceIndex);
251-
removeReadOnly(destIndex);
281+
cleanupMetadataBlocks(sourceIndex);
282+
cleanupMetadataBlocks(destIndex);
252283
}
253284

254285
public void testUpdateSettingsDefaultsRestored() {
@@ -428,10 +459,11 @@ public void testTsdbStartEndSet() throws Exception {
428459
// TODO check other IndexMetadata fields that need to be fixed after the fact
429460
// TODO what happens if don't have necessary perms for a given index?
430461

431-
private static void removeReadOnly(String index) {
462+
private static void cleanupMetadataBlocks(String index) {
432463
var settings = Settings.builder()
433-
.put(IndexMetadata.SETTING_READ_ONLY, false)
434-
.put(IndexMetadata.SETTING_BLOCKS_WRITE, false)
464+
.putNull(IndexMetadata.SETTING_READ_ONLY)
465+
.putNull(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE)
466+
.putNull(IndexMetadata.SETTING_BLOCKS_METADATA)
435467
.build();
436468
assertAcked(indicesAdmin().updateSettings(new UpdateSettingsRequest(settings, index)).actionGet());
437469
}

‎x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import java.util.Locale;
4141
import java.util.Map;
4242

43-
import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.READ_ONLY;
4443
import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.WRITE;
4544

4645
public class ReindexDataStreamIndexTransportAction extends HandledTransportAction<
@@ -93,13 +92,22 @@ protected void doExecute(
9392
);
9493
}
9594

95+
if (settingsBefore.getAsBoolean(IndexMetadata.SETTING_BLOCKS_READ, false)) {
96+
var errorMessage = String.format(Locale.ROOT, "Cannot reindex index [%s] which has a read block.", destIndexName);
97+
listener.onFailure(new ElasticsearchException(errorMessage));
98+
return;
99+
}
100+
if (settingsBefore.getAsBoolean(IndexMetadata.SETTING_BLOCKS_METADATA, false)) {
101+
var errorMessage = String.format(Locale.ROOT, "Cannot reindex index [%s] which has a metadata block.", destIndexName);
102+
listener.onFailure(new ElasticsearchException(errorMessage));
103+
return;
104+
}
105+
96106
SubscribableListener.<AcknowledgedResponse>newForked(l -> setBlockWrites(sourceIndexName, l, taskId))
97107
.<AcknowledgedResponse>andThen(l -> deleteDestIfExists(destIndexName, l, taskId))
98108
.<AcknowledgedResponse>andThen(l -> createIndex(sourceIndex, destIndexName, l, taskId))
99109
.<BulkByScrollResponse>andThen(l -> reindex(sourceIndexName, destIndexName, l, taskId))
100110
.<AcknowledgedResponse>andThen(l -> copyOldSourceSettingsToDest(settingsBefore, destIndexName, l, taskId))
101-
.<AddIndexBlockResponse>andThen(l -> addBlockIfFromSource(WRITE, settingsBefore, destIndexName, l, taskId))
102-
.<AddIndexBlockResponse>andThen(l -> addBlockIfFromSource(READ_ONLY, settingsBefore, destIndexName, l, taskId))
103111
.andThenApply(ignored -> new ReindexDataStreamIndexAction.Response(destIndexName))
104112
.addListener(listener);
105113
}
@@ -120,7 +128,8 @@ public void onResponse(AddIndexBlockResponse response) {
120128
@Override
121129
public void onFailure(Exception e) {
122130
if (e instanceof ClusterBlockException || e.getCause() instanceof ClusterBlockException) {
123-
// It's fine if block-writes is already set
131+
// Could fail with a cluster block exception if read-only or read-only-allow-delete is already set
132+
// In this case, we can proceed
124133
listener.onResponse(null);
125134
} else {
126135
listener.onFailure(e);
@@ -146,10 +155,12 @@ private void createIndex(
146155
) {
147156
logger.debug("Creating destination index [{}] for source index [{}]", destIndexName, sourceIndex.getIndex().getName());
148157

149-
// override read-only settings if they exist
150158
var removeReadOnlyOverride = Settings.builder()
159+
// remove read-only settings if they exist
151160
.putNull(IndexMetadata.SETTING_READ_ONLY)
161+
.putNull(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE)
152162
.putNull(IndexMetadata.SETTING_BLOCKS_WRITE)
163+
// settings to optimize reindex
153164
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
154165
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), -1)
155166
.build();
@@ -192,22 +203,29 @@ private void addBlockIfFromSource(
192203
}
193204
}
194205

206+
private void updateSettings(
207+
String index,
208+
Settings.Builder settings,
209+
ActionListener<AcknowledgedResponse> listener,
210+
TaskId parentTaskId
211+
) {
212+
var updateSettingsRequest = new UpdateSettingsRequest(settings.build(), index);
213+
updateSettingsRequest.setParentTask(parentTaskId);
214+
var errorMessage = String.format(Locale.ROOT, "Could not update settings on index [%s]", index);
215+
client.admin().indices().updateSettings(updateSettingsRequest, failIfNotAcknowledged(listener, errorMessage));
216+
}
217+
195218
private void copyOldSourceSettingsToDest(
196219
Settings settingsBefore,
197220
String destIndexName,
198221
ActionListener<AcknowledgedResponse> listener,
199222
TaskId parentTaskId
200223
) {
201224
logger.debug("Updating settings on destination index after reindex completes");
202-
203225
var settings = Settings.builder();
204226
copySettingOrUnset(settingsBefore, settings, IndexMetadata.SETTING_NUMBER_OF_REPLICAS);
205227
copySettingOrUnset(settingsBefore, settings, IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey());
206-
207-
var updateSettingsRequest = new UpdateSettingsRequest(settings.build(), destIndexName);
208-
updateSettingsRequest.setParentTask(parentTaskId);
209-
var errorMessage = String.format(Locale.ROOT, "Could not update settings on index [%s]", destIndexName);
210-
client.admin().indices().updateSettings(updateSettingsRequest, failIfNotAcknowledged(listener, errorMessage));
228+
updateSettings(destIndexName, settings, listener, parentTaskId);
211229
}
212230

213231
private static void copySettingOrUnset(Settings settingsBefore, Settings.Builder builder, String setting) {

0 commit comments

Comments
 (0)