Skip to content

Commit fa2fe4f

Browse files
nielsbaumannielsbauman
committed
Fix some lazy rollover code
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 9cd0db7 commit fa2fe4f

File tree

4 files changed

+102
-54
lines changed

4 files changed

+102
-54
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
@@ -276,6 +276,11 @@ public ClusterState executeTask(
276276

277277
final var rolloverIndexName = rolloverResult.rolloverIndexName();
278278
final var sourceIndexName = rolloverResult.sourceIndexName();
279+
logger.info(
280+
"rolling over data stream [{}] to index [{}] because it was marked for lazy rollover",
281+
dataStream.getName(),
282+
rolloverIndexName
283+
);
279284

280285
final var waitForActiveShardsTimeout = rolloverRequest.masterNodeTimeout().millis() < 0
281286
? 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
@@ -222,45 +222,17 @@ protected void masterOperation(
222222
final String trialRolloverIndexName = trialRolloverNames.rolloverName();
223223
MetadataCreateIndexService.validateIndexName(trialRolloverIndexName, projectMetadata, projectState.routingTable());
224224

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

266238
final IndexAbstraction rolloverTargetAbstraction = projectMetadata.getIndicesLookup().get(resolvedRolloverTarget.resource());
@@ -354,7 +326,7 @@ protected void masterOperation(
354326
false,
355327
false,
356328
false,
357-
rolloverRequest.isLazy()
329+
false
358330
);
359331

360332
// If this is a dry run, return with the results without invoking a cluster state update
@@ -383,6 +355,58 @@ protected void masterOperation(
383355
);
384356
}
385357

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

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

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -470,19 +470,33 @@ 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.isLazy(), equalTo(true));
498+
assertThat(rolloverResponse.isDryRun(), equalTo(true));
499+
}
486500
}
487501

488502
public void testLazyRolloverFails() throws Exception {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -842,8 +842,13 @@ public void onFailure(Exception e) {
842842

843843
void onRolloversBulkResponse(Collection<RolloverResponse> rolloverResponses) {
844844
for (RolloverResponse rolloverResponse : rolloverResponses) {
845-
if (rolloverResponse.isRolledOver() == false) {
846-
logger.warn("rollover of the [{}] index [{}] failed", getOrigin(), rolloverResponse.getOldIndex());
845+
if (rolloverResponse.isRolledOver() == false && rolloverResponse.isLazy() == false) {
846+
logger.warn(
847+
"rollover of the [{}] index [{}] failed, rollover response was [{}]",
848+
getOrigin(),
849+
rolloverResponse.getOldIndex(),
850+
rolloverResponse
851+
);
847852
}
848853
}
849854
}

0 commit comments

Comments
 (0)