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.

Sign up for GitHub

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

Move IndexLifecycleMetadata installation to put-lifecycle-action #31346

Merged
merged 2 commits into from
Jun 15, 2018
Merged

Move IndexLifecycleMetadata installation to put-lifecycle-action #31346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a12ddc2
Move IndexLifecycleMetadata installation to put-lifecycle-action
talevy Jun 14, 2018
7e02ffd
add null check for ilmMetadata in setpolicyaction
talevy Jun 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...ore/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
public class IndexLifecycleMetadata implements XPackMetaDataCustom {
public static final String TYPE = "index_lifecycle";
public static final ParseField POLICIES_FIELD = new ParseField("policies");
public static final IndexLifecycleMetadata EMPTY = new IndexLifecycleMetadata(Collections.emptySortedMap());

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<IndexLifecycleMetadata, Void> PARSER = new ConstructingObjectParser<>(
Expand Down
6 changes: 3 additions & 3 deletions .../index-lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import org.elasticsearch.xpack.indexlifecycle.action.RestPutLifecycleAction;
import org.elasticsearch.xpack.indexlifecycle.action.RestRetryAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportSetPolicyForIndexAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportDeleteLifcycleAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportDeleteLifecycleAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportExplainLifecycleAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportGetLifecycleAction;
import org.elasticsearch.xpack.indexlifecycle.action.TransportMoveToStepAction;
Expand Down Expand Up @@ -124,7 +124,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
return emptyList();
}
indexLifecycleInitialisationService
.set(new IndexLifecycleService(settings, client, clusterService, getClock(), threadPool, System::currentTimeMillis));
.set(new IndexLifecycleService(settings, client, clusterService, getClock(), System::currentTimeMillis));
return Collections.singletonList(indexLifecycleInitialisationService.get());
}

Expand Down Expand Up @@ -164,7 +164,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
return Arrays.asList(
new ActionHandler<>(PutLifecycleAction.INSTANCE, TransportPutLifecycleAction.class),
new ActionHandler<>(GetLifecycleAction.INSTANCE, TransportGetLifecycleAction.class),
new ActionHandler<>(DeleteLifecycleAction.INSTANCE, TransportDeleteLifcycleAction.class),
new ActionHandler<>(DeleteLifecycleAction.INSTANCE, TransportDeleteLifecycleAction.class),
new ActionHandler<>(ExplainLifecycleAction.INSTANCE, TransportExplainLifecycleAction.class),
new ActionHandler<>(SetPolicyForIndexAction.INSTANCE, TransportSetPolicyForIndexAction.class),
new ActionHandler<>(MoveToStepAction.INSTANCE, TransportMoveToStepAction.class),
Expand Down
40 changes: 6 additions & 34 deletions ...lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterStateApplier;
import org.elasticsearch.cluster.ClusterStateListener;
import org.elasticsearch.cluster.ClusterStateUpdateTask;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata;
import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy;
import org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings;
Expand All @@ -29,7 +26,6 @@

import java.io.Closeable;
import java.time.Clock;
import java.util.Collections;
import java.util.function.LongSupplier;

/**
Expand All @@ -44,18 +40,15 @@ public class IndexLifecycleService extends AbstractComponent
private final PolicyStepsRegistry policyRegistry;
private Client client;
private ClusterService clusterService;
private ThreadPool threadPool;
private LongSupplier nowSupplier;
private SchedulerEngine.Job scheduledJob;
private IndexLifecycleRunner lifecycleRunner;

public IndexLifecycleService(Settings settings, Client client, ClusterService clusterService, Clock clock,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ThreadPool is no longer used since the service does not issue a cluster-state-update to install metadata anymore

ThreadPool threadPool, LongSupplier nowSupplier) {
public IndexLifecycleService(Settings settings, Client client, ClusterService clusterService, Clock clock, LongSupplier nowSupplier) {
super(settings);
this.client = client;
this.clusterService = clusterService;
this.clock = clock;
this.threadPool = threadPool;
this.nowSupplier = nowSupplier;
this.scheduledJob = null;
this.policyRegistry = new PolicyStepsRegistry();
Expand Down Expand Up @@ -91,19 +84,17 @@ public PolicyStepsRegistry getPolicyRegistry() {

@Override
public void clusterChanged(ClusterChangedEvent event) {
if (event.localNodeMaster()) { // only act if we are master, otherwise keep idle until elected
IndexLifecycleMetadata lifecycleMetadata = event.state().metaData().custom(IndexLifecycleMetadata.TYPE);
IndexLifecycleMetadata lifecycleMetadata = event.state().metaData().custom(IndexLifecycleMetadata.TYPE);
if (event.localNodeMaster() && lifecycleMetadata != null) {
TimeValue pollInterval = LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING
.get(event.state().getMetaData().settings());
TimeValue previousPollInterval = LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING
.get(event.previousState().getMetaData().settings());

boolean pollIntervalSettingChanged = !pollInterval.equals(previousPollInterval);

if (lifecycleMetadata == null) { // no lifecycle metadata, install initial empty metadata state
lifecycleMetadata = new IndexLifecycleMetadata(Collections.emptySortedMap());
installMetadata(lifecycleMetadata);
} else if (scheduler.get() == null) { // metadata installed and scheduler should be kicked off. start your engines.

if (scheduler.get() == null) { // metadata installed and scheduler should be kicked off. start your engines.
scheduler.set(new SchedulerEngine(clock));
scheduler.get().register(this);
scheduleJob(pollInterval);
Expand Down Expand Up @@ -151,26 +142,7 @@ public void triggered(SchedulerEngine.Event event) {
}
}

private void installMetadata(IndexLifecycleMetadata lifecycleMetadata) {
threadPool.executor(ThreadPool.Names.GENERIC)
.execute(() -> clusterService.submitStateUpdateTask("install-index-lifecycle-metadata", new ClusterStateUpdateTask() {
@Override
public ClusterState execute(ClusterState currentState) throws Exception {
ClusterState.Builder builder = new ClusterState.Builder(currentState);
MetaData.Builder metadataBuilder = MetaData.builder(currentState.metaData());
metadataBuilder.putCustom(IndexLifecycleMetadata.TYPE, lifecycleMetadata);
builder.metaData(metadataBuilder.build());
return builder.build();
}

@Override
public void onFailure(String source, Exception e) {
logger.error("unable to install index lifecycle metadata", e);
}
}));
}

public void triggerPolicies(ClusterState clusterState, boolean fromClusterStateChange) {
void triggerPolicies(ClusterState clusterState, boolean fromClusterStateChange) {
// loop through all indices in cluster state and filter for ones that are
// managed by the Index Lifecycle Service they have a index.lifecycle.name setting
// associated to a policy
Expand Down
2 changes: 2 additions & 0 deletions ...x-lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/PolicyStepsRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ Map<String, Map<Step.StepKey, Step>> getStepMap() {
@SuppressWarnings({ "unchecked", "rawtypes" })
public void update(ClusterState currentState, Client client, LongSupplier nowSupplier) {
IndexLifecycleMetadata meta = currentState.metaData().custom(IndexLifecycleMetadata.TYPE);
assert meta != null : "IndexLifecycleMetadata cannot be null when updating the policy steps registry";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen this pattern of assertion elsewhere in the codebase. I am not sure how I feel about it, but I chose to try it out here to be self-documenting. Currently this method is only ever called once, and it is behind a null check


Diff<Map<String, LifecyclePolicyMetadata>> diff = DiffableUtils.diff(lifecyclePolicyMap, meta.getPolicyMetadatas(),
DiffableUtils.getStringKeySerializer());
DiffableUtils.MapDiff<String, LifecyclePolicyMetadata, DiffableUtils.KeySerializer<String>> mapDiff = (DiffableUtils.MapDiff) diff;
Expand Down
12 changes: 7 additions & 5 deletions ...action/TransportDeleteLifcycleAction.java → ...ction/TransportDeleteLifecycleAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
import java.util.SortedMap;
import java.util.TreeMap;

public class TransportDeleteLifcycleAction extends TransportMasterNodeAction<Request, Response> {
public class TransportDeleteLifecycleAction extends TransportMasterNodeAction<Request, Response> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed an existing typo


@Inject
public TransportDeleteLifcycleAction(Settings settings, TransportService transportService, ClusterService clusterService,
ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
public TransportDeleteLifecycleAction(Settings settings, TransportService transportService, ClusterService clusterService,
ThreadPool threadPool, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, DeleteLifecycleAction.NAME, transportService, clusterService, threadPool, actionFilters,
indexNameExpressionResolver, Request::new);
}
Expand Down Expand Up @@ -74,7 +75,8 @@ public ClusterState execute(ClusterState currentState) {
}
ClusterState.Builder newState = ClusterState.builder(currentState);
IndexLifecycleMetadata currentMetadata = currentState.metaData().custom(IndexLifecycleMetadata.TYPE);
if (currentMetadata.getPolicyMetadatas().containsKey(request.getPolicyName()) == false) {
if (currentMetadata == null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since this is an anonymous class, we do not have good unit test coverage, so I've added a yaml test to walk this branch

|| currentMetadata.getPolicyMetadatas().containsKey(request.getPolicyName()) == false) {
throw new ResourceNotFoundException("Lifecycle policy not found: {}", request.getPolicyName());
}
SortedMap<String, LifecyclePolicyMetadata> newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas());
Expand All @@ -91,4 +93,4 @@ public ClusterState execute(ClusterState currentState) {
protected ClusterBlockException checkBlock(Request request, ClusterState state) {
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
}
}
}
9 changes: 8 additions & 1 deletion .../main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
import java.util.TreeMap;
import java.util.stream.Collectors;

/**
* This class is responsible for bootstrapping {@link IndexLifecycleMetadata} into the cluster-state, as well
* as adding the desired new policy to be inserted.
*/
public class TransportPutLifecycleAction extends TransportMasterNodeAction<Request, Response> {

@Inject
Expand Down Expand Up @@ -64,6 +68,9 @@ protected Response newResponse(boolean acknowledged) {
public ClusterState execute(ClusterState currentState) throws Exception {
ClusterState.Builder newState = ClusterState.builder(currentState);
IndexLifecycleMetadata currentMetadata = currentState.metaData().custom(IndexLifecycleMetadata.TYPE);
if (currentMetadata == null) { // first time using index-lifecycle feature, bootstrap metadata
Copy link
Contributor Author

Choose a reason for hiding this comment

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

existing yaml tests should walk this branch

currentMetadata = IndexLifecycleMetadata.EMPTY;
}
if (currentMetadata.getPolicyMetadatas().containsKey(request.getPolicy().getName())) {
throw new ResourceAlreadyExistsException("Lifecycle policy already exists: {}",
request.getPolicy().getName());
Expand All @@ -87,4 +94,4 @@ public ClusterState execute(ClusterState currentState) throws Exception {
protected ClusterBlockException checkBlock(Request request, ClusterState state) {
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
}
}
}
Loading