Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ILM] Version the order of the actions in a phase. #97745

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
15 changes: 11 additions & 4 deletions docs/reference/ilm/apis/explain.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ that the index is managed and in the `new` phase:
"action": "complete"
"action_time_millis": 1538475653317, <8>
"step": "complete",
"step_time_millis": 1538475653317 <9>
"step_time_millis": 1538475653317, <9>
"actions_order_version": 2 <10>
}
}
}
Expand All @@ -141,6 +142,9 @@ the index via the `max_age`)
<7> When the index entered the current phase
<8> When the index entered the current action
<9> When the index entered the current step
<10> The version of the actions order this phases is executing. Once an index enters a phase
it will execute the actions in the order they were defined when entering the phase, regardless
of any new order having been made available in the system after a potential upgrade.

Once the policy is running on the index, the response includes a
`phase_execution` object that shows the definition of the current phase.
Expand Down Expand Up @@ -182,7 +186,8 @@ phase completes.
"version": 3, <2>
"modified_date": "2018-10-15T13:21:41.576Z", <3>
"modified_date_in_millis": 1539609701576 <4>
}
},
"actions_order_version": 2
}
}
}
Expand Down Expand Up @@ -247,7 +252,8 @@ information for the step that's being performed on the index.
"version": 2,
"modified_date": "2018-10-15T13:20:02.489Z",
"modified_date_in_millis": 1539609602489
}
},
"actions_order_version": 2
}
}
}
Expand Down Expand Up @@ -308,7 +314,8 @@ the case.
"version": 3,
"modified_date": "2018-10-15T13:21:41.576Z",
"modified_date_in_millis": 1539609701576
}
},
"actions_order_version": 2
}
}
}
Expand Down
1 change: 1 addition & 0 deletions docs/reference/ilm/error-handling.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ Which returns the following information:
"version" : 1,
"modified_date_in_millis" : 1541717264230
}
"actions_order_version": 2
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions docs/reference/ilm/ilm-tutorial.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,9 @@ is met.
}
},
"version": 1,
"modified_date_in_millis": 1539609701576
}
"modified_date_in_millis": 1539609701576,
},
"actions_order_version": 2
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/TransportVersion.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ private static TransportVersion registerTransportVersion(int id, String uniqueId
public static final TransportVersion V_8_500_037 = registerTransportVersion(8_500_037, "d76a4f22-8878-43e0-acfa-15e452195fa7");
public static final TransportVersion V_8_500_038 = registerTransportVersion(8_500_038, "9ef93580-feae-409f-9989-b49e411ca7a9");
public static final TransportVersion V_8_500_039 = registerTransportVersion(8_500_039, "c23722d7-6139-4cf2-b8a1-600fbd4ec359");
// Introduced for the actions_order_version in the explain ILM API
public static final TransportVersion V_8_500_040 = registerTransportVersion(8_500_040, "E3C215E9-6F54-465A-A31A-CBC4A65D649B");

private static class CurrentHolder {
private static final TransportVersion CURRENT = findCurrent(V_8_500_039);
private static final TransportVersion CURRENT = findCurrent(V_8_500_040);

// finds the pluggable current version, or uses the given fallback
private static TransportVersion findCurrent(TransportVersion fallback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,16 @@ public record LifecycleExecutionState(
String snapshotName,
String shrinkIndexName,
String snapshotIndexName,
String downsampleIndexName
String downsampleIndexName,
Integer actionsOrderVersion
) {

public LifecycleExecutionState {
if (actionsOrderVersion == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also enforce that it's > 0, to avoid weird -1 issues

actionsOrderVersion = 1;
}
}

public static final String ILM_CUSTOM_METADATA_KEY = "ilm";

private static final String PHASE = "phase";
Expand All @@ -58,6 +65,7 @@ public record LifecycleExecutionState(
private static final String SNAPSHOT_INDEX_NAME = "snapshot_index_name";
private static final String SHRINK_INDEX_NAME = "shrink_index_name";
private static final String DOWNSAMPLE_INDEX_NAME = "rollup_index_name";
private static final String ACTIONS_ORDER_VERSION = "actions_order_version";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should go with action_order_version and avoid the awkward plural-action-but-singular-version noun


public static final LifecycleExecutionState EMPTY_STATE = LifecycleExecutionState.builder().build();

Expand All @@ -82,7 +90,8 @@ public static Builder builder(LifecycleExecutionState state) {
.setShrinkIndexName(state.shrinkIndexName)
.setSnapshotIndexName(state.snapshotIndexName)
.setDownsampleIndexName(state.downsampleIndexName)
.setStepTime(state.stepTime);
.setStepTime(state.stepTime)
.setActionsOrderVersion(state.actionsOrderVersion);
}

public static LifecycleExecutionState fromCustomMetadata(Map<String, String> customData) {
Expand Down Expand Up @@ -191,6 +200,10 @@ public static LifecycleExecutionState fromCustomMetadata(Map<String, String> cus
if (downsampleIndexName != null) {
builder.setDownsampleIndexName(downsampleIndexName);
}
String actionsOrderVersion = customData.get(ACTIONS_ORDER_VERSION);
if (actionsOrderVersion != null) {
builder.setActionsOrderVersion(Integer.parseInt(actionsOrderVersion));
}
return builder.build();
}

Expand Down Expand Up @@ -253,6 +266,9 @@ public Map<String, String> asMap() {
if (downsampleIndexName != null) {
result.put(DOWNSAMPLE_INDEX_NAME, downsampleIndexName);
}
if (actionsOrderVersion != null) {
result.put(ACTIONS_ORDER_VERSION, String.valueOf(actionsOrderVersion));
}
return Collections.unmodifiableMap(result);
}

Expand All @@ -274,6 +290,7 @@ public static class Builder {
private String shrinkIndexName;
private String snapshotIndexName;
private String downsampleIndexName;
private Integer actionsOrderVersion;

public Builder setPhase(String phase) {
this.phase = phase;
Expand Down Expand Up @@ -360,6 +377,11 @@ public Builder setDownsampleIndexName(String downsampleIndexName) {
return this;
}

public Builder setActionsOrderVersion(Integer actionsOrderVersion) {
this.actionsOrderVersion = actionsOrderVersion;
return this;
}

public LifecycleExecutionState build() {
return new LifecycleExecutionState(
phase,
Expand All @@ -378,7 +400,8 @@ public LifecycleExecutionState build() {
snapshotName,
shrinkIndexName,
snapshotIndexName,
downsampleIndexName
downsampleIndexName,
actionsOrderVersion
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.function.Supplier;
import java.util.stream.Stream;

import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleActionsRegistry.VERSION_ONE;

public class IndexLifecycleExplainResponse implements ToXContentObject, Writeable {

private static final ParseField INDEX_FIELD = new ParseField("index");
Expand Down Expand Up @@ -54,6 +56,7 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl
private static final ParseField REPOSITORY_NAME = new ParseField("repository_name");
private static final ParseField SHRINK_INDEX_NAME = new ParseField("shrink_index_name");
private static final ParseField SNAPSHOT_NAME = new ParseField("snapshot_name");
private static ParseField ACTIONS_ORDER_VERSION_FIELD = new ParseField("actions_order_version");

public static final ConstructingObjectParser<IndexLifecycleExplainResponse, Void> PARSER = new ConstructingObjectParser<>(
"index_lifecycle_explain_response",
Expand All @@ -76,9 +79,10 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl
(String) a[17],
(String) a[18],
(BytesReference) a[11],
(PhaseExecutionInfo) a[12]
(PhaseExecutionInfo) a[12],
// a[13] == "age"
// a[20] == "time_since_index_creation"
(Integer) a[21]
)
);
static {
Expand Down Expand Up @@ -111,6 +115,7 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), SHRINK_INDEX_NAME);
PARSER.declareLong(ConstructingObjectParser.optionalConstructorArg(), INDEX_CREATION_DATE_MILLIS_FIELD);
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), TIME_SINCE_INDEX_CREATION_FIELD);
PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), ACTIONS_ORDER_VERSION_FIELD);
}

private final String index;
Expand All @@ -132,6 +137,7 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl
private final String repositoryName;
private final String snapshotName;
private final String shrinkIndexName;
private final Integer actionsOrderVersion;

Supplier<Long> nowSupplier = System::currentTimeMillis; // Can be changed for testing

Expand All @@ -153,7 +159,8 @@ public static IndexLifecycleExplainResponse newManagedIndexResponse(
String snapshotName,
String shrinkIndexName,
BytesReference stepInfo,
PhaseExecutionInfo phaseExecutionInfo
PhaseExecutionInfo phaseExecutionInfo,
Integer actionsOrderVersion
) {
return new IndexLifecycleExplainResponse(
index,
Expand All @@ -174,7 +181,8 @@ public static IndexLifecycleExplainResponse newManagedIndexResponse(
snapshotName,
shrinkIndexName,
stepInfo,
phaseExecutionInfo
phaseExecutionInfo,
actionsOrderVersion
);
}

Expand All @@ -198,6 +206,7 @@ public static IndexLifecycleExplainResponse newUnmanagedIndexResponse(String ind
null,
null,
null,
null,
null
);
}
Expand All @@ -221,7 +230,8 @@ private IndexLifecycleExplainResponse(
String snapshotName,
String shrinkIndexName,
BytesReference stepInfo,
PhaseExecutionInfo phaseExecutionInfo
PhaseExecutionInfo phaseExecutionInfo,
Integer actionsOrderVersion
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We treat this as non-nullable elsewhere with the check in the state against null and setting it to 1. Should we remove the nullification here and make it a regular int, enforcing it to be 1 if it's unset?

) {
if (managedByILM) {
if (policyName == null) {
Expand Down Expand Up @@ -283,6 +293,7 @@ private IndexLifecycleExplainResponse(
this.repositoryName = repositoryName;
this.snapshotName = snapshotName;
this.shrinkIndexName = shrinkIndexName;
this.actionsOrderVersion = actionsOrderVersion;
}

public IndexLifecycleExplainResponse(StreamInput in) throws IOException {
Expand Down Expand Up @@ -310,6 +321,11 @@ public IndexLifecycleExplainResponse(StreamInput in) throws IOException {
} else {
indexCreationDate = null;
}
if (in.getTransportVersion().onOrAfter(TransportVersion.V_8_500_040)) {
actionsOrderVersion = in.readOptionalInt();
} else {
actionsOrderVersion = VERSION_ONE;
}
} else {
policyName = null;
lifecycleDate = null;
Expand All @@ -328,6 +344,7 @@ public IndexLifecycleExplainResponse(StreamInput in) throws IOException {
snapshotName = null;
shrinkIndexName = null;
indexCreationDate = null;
actionsOrderVersion = null;
}
}

Expand Down Expand Up @@ -355,6 +372,9 @@ public void writeTo(StreamOutput out) throws IOException {
if (out.getTransportVersion().onOrAfter(TransportVersion.V_8_1_0)) {
out.writeOptionalLong(indexCreationDate);
}
if (out.getTransportVersion().onOrAfter(TransportVersion.V_8_500_040)) {
out.writeOptionalInt(actionsOrderVersion);
}
}
}

Expand Down Expand Up @@ -450,6 +470,10 @@ public String getShrinkIndexName() {
return shrinkIndexName;
}

public Integer getActionsOrderVersion() {
return actionsOrderVersion;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
Expand Down Expand Up @@ -514,6 +538,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (phaseExecutionInfo != null) {
builder.field(PHASE_EXECUTION_INFO.getPreferredName(), phaseExecutionInfo);
}
if (actionsOrderVersion != null) {
builder.field(ACTIONS_ORDER_VERSION_FIELD.getPreferredName(), actionsOrderVersion);
}
}
builder.endObject();
return builder;
Expand All @@ -540,7 +567,8 @@ public int hashCode() {
snapshotName,
shrinkIndexName,
stepInfo,
phaseExecutionInfo
phaseExecutionInfo,
actionsOrderVersion
);
}

Expand Down Expand Up @@ -571,7 +599,8 @@ public boolean equals(Object obj) {
&& Objects.equals(snapshotName, other.snapshotName)
&& Objects.equals(shrinkIndexName, other.shrinkIndexName)
&& Objects.equals(stepInfo, other.stepInfo)
&& Objects.equals(phaseExecutionInfo, other.phaseExecutionInfo);
&& Objects.equals(phaseExecutionInfo, other.phaseExecutionInfo)
&& Objects.equals(actionsOrderVersion, other.actionsOrderVersion);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
*
* @param client The Elasticsearch Client to use during execution of {@link AsyncActionStep}
* and {@link AsyncWaitStep} steps.
* @param actionsOrderVersion the version of the actions order to use
* @param licenseState The license state to use in actions and steps
* @return The list of {@link Step} objects in order of their execution.
*/
public List<Step> toSteps(Client client, XPackLicenseState licenseState) {
public List<Step> toSteps(Client client, int actionsOrderVersion, XPackLicenseState licenseState) {
List<Step> steps = new ArrayList<>();
List<Phase> orderedPhases = type.getOrderedPhases(phases);
ListIterator<Phase> phaseIterator = orderedPhases.listIterator(orderedPhases.size());
Expand All @@ -233,7 +234,7 @@ public List<Step> toSteps(Client client, XPackLicenseState licenseState) {
}

phase = previousPhase;
List<LifecycleAction> orderedActions = type.getOrderedActions(phase);
List<LifecycleAction> orderedActions = type.getOrderedActions(phase, actionsOrderVersion);
ListIterator<LifecycleAction> actionIterator = orderedActions.listIterator(orderedActions.size());
// add steps for each action, in reverse
while (actionIterator.hasPrevious()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public interface LifecycleType extends NamedWriteable {
*/
List<Phase> getOrderedPhases(Map<String, Phase> phases);

List<LifecycleAction> getOrderedActions(Phase phase);
List<LifecycleAction> getOrderedActions(Phase phase, int orderVersion);

/**
* validates whether the specified <code>phases</code> are valid for this
Expand All @@ -31,4 +31,6 @@ public interface LifecycleType extends NamedWriteable {
* if a specific phase or lack of a specific phase is invalid.
*/
void validate(Collection<Phase> phases);

int getLatestActionsOrderVersion();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add javadocs for this?

}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public static void refreshPhaseDefinition(
updatedPolicy.getModifiedDate()
);

// we're not updating the actions order version here as we're still executing the same phase (and the step keys are identical)
LifecycleExecutionState newExState = LifecycleExecutionState.builder(currentExState)
.setPhaseDefinition(Strings.toString(pei, false, false))
.build();
Expand Down Expand Up @@ -192,7 +193,9 @@ public static boolean isIndexPhaseDefinitionUpdatable(
final Step.StepKey currentStepKey = Step.getCurrentStepKey(executionState);
final String currentPhase = currentStepKey.phase();

final Set<Step.StepKey> newStepKeys = newPolicy.toSteps(client, licenseState)
// reading the same order version of the new policy as if we refresh the cached phase, being the same phase
// as it's currently executing, we won't update the actions order version
final Set<Step.StepKey> newStepKeys = newPolicy.toSteps(client, executionState.actionsOrderVersion(), licenseState)
.stream()
.map(Step::getKey)
.collect(Collectors.toCollection(LinkedHashSet::new));
Expand Down
Loading