Skip to content

Commit 9544204

Browse files
nielsbaumannielsbauman
authored
Fix some lazy rollover code (#124153)
The main goal of the PR is to fix the "rollover failed" log in the `IndexTemplateRegistry`. We were logging this on every upgrade because `rolloverResponse.isRolledOver()` is always `false` when we do a lazy rollover request - which is the default for index template updates. While I was looking at this, I noticed we don't actually return early when we process a lazy rollover _dry run_. The end result happened to be ok (i.e. side-effects and response), but I figured I might as well fix the behavior to return early. Finally, we add an `INFO` log in the lazy rollover action to aid the debugging process.
1 parent b218f69 commit 9544204

File tree

4 files changed

+104
-55
lines changed

4 files changed

+104
-55
lines changed

‎server/src/main/java/org/elasticsearch/action/admin/indices/rollover/LazyRolloverAction.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,11 @@ public ClusterState executeTask(
268268

269269
final var rolloverIndexName = rolloverResult.rolloverIndexName();
270270
final var sourceIndexName = rolloverResult.sourceIndexName();
271+
logger.info(
272+
"rolling over data stream [{}] to index [{}] because it was marked for lazy rollover",
273+
dataStream.getName(),
274+
rolloverIndexName
275+
);
271276

272277
final var waitForActiveShardsTimeout = rolloverRequest.masterNodeTimeout().millis() < 0
273278
? null

‎server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java‎

Lines changed: 63 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -221,45 +221,17 @@ protected void masterOperation(
221221
final String trialRolloverIndexName = trialRolloverNames.rolloverName();
222222
MetadataCreateIndexService.validateIndexName(trialRolloverIndexName, projectMetadata, projectState.routingTable());
223223

224-
boolean isDataStream = projectMetadata.dataStreams().containsKey(resolvedRolloverTarget.resource());
225224
if (rolloverRequest.isLazy()) {
226-
if (isDataStream == false || rolloverRequest.getConditions().hasConditions()) {
227-
String message;
228-
if (isDataStream) {
229-
message = "Lazy rollover can be used only without any conditions."
230-
+ " Please remove the conditions from the request body or the query parameter 'lazy'.";
231-
} else if (rolloverRequest.getConditions().hasConditions() == false) {
232-
message = "Lazy rollover can be applied only on a data stream." + " Please remove the query parameter 'lazy'.";
233-
} else {
234-
message = "Lazy rollover can be applied only on a data stream with no conditions."
235-
+ " Please remove the query parameter 'lazy'.";
236-
}
237-
listener.onFailure(new IllegalArgumentException(message));
238-
return;
239-
}
240-
if (rolloverRequest.isDryRun() == false) {
241-
metadataDataStreamsService.setRolloverOnWrite(
242-
projectState.projectId(),
243-
resolvedRolloverTarget.resource(),
244-
true,
245-
targetFailureStore,
246-
rolloverRequest.ackTimeout(),
247-
rolloverRequest.masterNodeTimeout(),
248-
listener.map(
249-
response -> new RolloverResponse(
250-
trialSourceIndexName,
251-
trialRolloverIndexName,
252-
Map.of(),
253-
false,
254-
false,
255-
response.isAcknowledged(),
256-
false,
257-
response.isAcknowledged()
258-
)
259-
)
260-
);
261-
return;
262-
}
225+
markForLazyRollover(
226+
rolloverRequest,
227+
listener,
228+
projectMetadata,
229+
resolvedRolloverTarget,
230+
targetFailureStore,
231+
trialSourceIndexName,
232+
trialRolloverIndexName
233+
);
234+
return;
263235
}
264236

265237
final IndexAbstraction rolloverTargetAbstraction = projectMetadata.getIndicesLookup().get(resolvedRolloverTarget.resource());
@@ -353,7 +325,7 @@ protected void masterOperation(
353325
false,
354326
false,
355327
false,
356-
rolloverRequest.isLazy()
328+
false
357329
);
358330

359331
// If this is a dry run, return with the results without invoking a cluster state update
@@ -382,6 +354,58 @@ protected void masterOperation(
382354
);
383355
}
384356

357+
private void markForLazyRollover(
358+
RolloverRequest rolloverRequest,
359+
ActionListener<RolloverResponse> listener,
360+
ProjectMetadata projectMetadata,
361+
ResolvedExpression resolvedRolloverTarget,
362+
boolean targetFailureStore,
363+
String trialSourceIndexName,
364+
String trialRolloverIndexName
365+
) {
366+
boolean isDataStream = projectMetadata.dataStreams().containsKey(resolvedRolloverTarget.resource());
367+
if (isDataStream == false || rolloverRequest.getConditions().hasConditions()) {
368+
String message;
369+
if (isDataStream) {
370+
message = "Lazy rollover can be used only without any conditions."
371+
+ " Please remove the conditions from the request body or the query parameter 'lazy'.";
372+
} else if (rolloverRequest.getConditions().hasConditions() == false) {
373+
message = "Lazy rollover can be applied only on a data stream. Please remove the query parameter 'lazy'.";
374+
} else {
375+
message = "Lazy rollover can be applied only on a data stream with no conditions."
376+
+ " Please remove the query parameter 'lazy'.";
377+
}
378+
listener.onFailure(new IllegalArgumentException(message));
379+
return;
380+
}
381+
if (rolloverRequest.isDryRun()) {
382+
listener.onResponse(
383+
new RolloverResponse(trialSourceIndexName, trialRolloverIndexName, Map.of(), true, false, false, false, true)
384+
);
385+
return;
386+
}
387+
metadataDataStreamsService.setRolloverOnWrite(
388+
projectMetadata.id(),
389+
resolvedRolloverTarget.resource(),
390+
true,
391+
targetFailureStore,
392+
rolloverRequest.ackTimeout(),
393+
rolloverRequest.masterNodeTimeout(),
394+
listener.map(
395+
response -> new RolloverResponse(
396+
trialSourceIndexName,
397+
trialRolloverIndexName,
398+
Map.of(),
399+
false,
400+
false,
401+
response.isAcknowledged(),
402+
false,
403+
true
404+
)
405+
)
406+
);
407+
}
408+
385409
private void initializeFailureStore(
386410
ProjectId projectId,
387411
RolloverRequest rolloverRequest,

‎server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -470,19 +470,39 @@ public void testLazyRollover() throws Exception {
470470
mockMetadataDataStreamService,
471471
dataStreamAutoShardingService
472472
);
473-
final PlainActionFuture<RolloverResponse> future = new PlainActionFuture<>();
474-
RolloverRequest rolloverRequest = new RolloverRequest("logs-ds", null);
475-
rolloverRequest.lazy(true);
476-
transportRolloverAction.masterOperation(mock(CancellableTask.class), rolloverRequest, stateBefore, future);
477-
RolloverResponse rolloverResponse = future.actionGet();
478-
assertThat(rolloverResponse.getOldIndex(), equalTo(".ds-logs-ds-000001"));
479-
assertThat(rolloverResponse.getNewIndex(), Matchers.startsWith(".ds-logs-ds-"));
480-
assertThat(rolloverResponse.getNewIndex(), Matchers.endsWith("-000002"));
481-
assertThat(rolloverResponse.isLazy(), equalTo(true));
482-
assertThat(rolloverResponse.isDryRun(), equalTo(false));
483-
assertThat(rolloverResponse.isRolledOver(), equalTo(false));
484-
assertThat(rolloverResponse.getConditionStatus().size(), equalTo(0));
485-
assertThat(rolloverResponse.isAcknowledged(), is(true));
473+
{
474+
// Regular lazy rollover
475+
final PlainActionFuture<RolloverResponse> future = new PlainActionFuture<>();
476+
RolloverRequest rolloverRequest = new RolloverRequest("logs-ds", null);
477+
rolloverRequest.lazy(true);
478+
transportRolloverAction.masterOperation(mock(CancellableTask.class), rolloverRequest, stateBefore, future);
479+
RolloverResponse rolloverResponse = future.actionGet();
480+
assertThat(rolloverResponse.getOldIndex(), equalTo(".ds-logs-ds-000001"));
481+
assertThat(rolloverResponse.getNewIndex(), Matchers.startsWith(".ds-logs-ds-"));
482+
assertThat(rolloverResponse.getNewIndex(), Matchers.endsWith("-000002"));
483+
assertThat(rolloverResponse.isLazy(), equalTo(true));
484+
assertThat(rolloverResponse.isDryRun(), equalTo(false));
485+
assertThat(rolloverResponse.isRolledOver(), equalTo(false));
486+
assertThat(rolloverResponse.getConditionStatus().size(), equalTo(0));
487+
assertThat(rolloverResponse.isAcknowledged(), is(true));
488+
}
489+
{
490+
// Dry-run lazy rollover
491+
final PlainActionFuture<RolloverResponse> future = new PlainActionFuture<>();
492+
RolloverRequest rolloverRequest = new RolloverRequest("logs-ds", null);
493+
rolloverRequest.lazy(true);
494+
rolloverRequest.dryRun(true);
495+
transportRolloverAction.masterOperation(mock(CancellableTask.class), rolloverRequest, stateBefore, future);
496+
RolloverResponse rolloverResponse = future.actionGet();
497+
assertThat(rolloverResponse.getOldIndex(), equalTo(".ds-logs-ds-000001"));
498+
assertThat(rolloverResponse.getNewIndex(), Matchers.startsWith(".ds-logs-ds-"));
499+
assertThat(rolloverResponse.getNewIndex(), Matchers.endsWith("-000002"));
500+
assertThat(rolloverResponse.isLazy(), equalTo(true));
501+
assertThat(rolloverResponse.isDryRun(), equalTo(true));
502+
assertThat(rolloverResponse.isRolledOver(), equalTo(false));
503+
assertThat(rolloverResponse.getConditionStatus().size(), equalTo(0));
504+
assertThat(rolloverResponse.isAcknowledged(), is(false));
505+
}
486506
}
487507

488508
public void testLazyRolloverFails() throws Exception {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.cluster.metadata.Template;
3535
import org.elasticsearch.cluster.node.DiscoveryNode;
3636
import org.elasticsearch.cluster.service.ClusterService;
37+
import org.elasticsearch.common.Strings;
3738
import org.elasticsearch.common.regex.Regex;
3839
import org.elasticsearch.common.settings.Settings;
3940
import org.elasticsearch.core.Nullable;
@@ -842,9 +843,8 @@ public void onFailure(Exception e) {
842843

843844
void onRolloversBulkResponse(Collection<RolloverResponse> rolloverResponses) {
844845
for (RolloverResponse rolloverResponse : rolloverResponses) {
845-
if (rolloverResponse.isRolledOver() == false) {
846-
logger.warn("rollover of the [{}] index [{}] failed", getOrigin(), rolloverResponse.getOldIndex());
847-
}
846+
assert rolloverResponse.isLazy() && rolloverResponse.isRolledOver() == false
847+
: Strings.format("Expected rollover of the [%s] index [%s] to be lazy", getOrigin(), rolloverResponse.getOldIndex());
848848
}
849849
}
850850

0 commit comments

Comments
 (0)