-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
Make DLM stats and DLM error store project-aware #124810
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -784,21 +784,22 @@ static List<Index> getTargetIndices( | |
*/ | ||
private void clearErrorStoreForUnmanagedIndices(DataStream dataStream) { | ||
Metadata metadata = clusterService.state().metadata(); | ||
for (String indexName : errorStore.getAllIndices()) { | ||
final var projectId = metadata.getProject().id(); | ||
for (String indexName : errorStore.getAllIndices(projectId)) { | ||
IndexAbstraction indexAbstraction = metadata.getProject().getIndicesLookup().get(indexName); | ||
DataStream parentDataStream = indexAbstraction != null ? indexAbstraction.getParentDataStream() : null; | ||
if (indexAbstraction == null || parentDataStream == null) { | ||
logger.trace( | ||
"Clearing recorded error for index [{}] because the index doesn't exist or is not a data stream backing index anymore", | ||
indexName | ||
); | ||
errorStore.clearRecordedError(indexName); | ||
errorStore.clearRecordedError(projectId, indexName); | ||
} else if (parentDataStream.getName().equals(dataStream.getName())) { | ||
// we're only verifying the indices that pertain to this data stream | ||
IndexMetadata indexMeta = metadata.getProject().index(indexName); | ||
if (dataStream.isIndexManagedByDataStreamLifecycle(indexMeta.getIndex(), metadata.getProject()::index) == false) { | ||
logger.trace("Clearing recorded error for index [{}] because the index is not managed by DSL anymore", indexName); | ||
errorStore.clearRecordedError(indexName); | ||
errorStore.clearRecordedError(projectId, indexName); | ||
} | ||
} | ||
} | ||
|
@@ -866,7 +867,7 @@ private Index maybeExecuteRollover(ClusterState state, DataStream dataStream, bo | |
if (latestDataStream.getWriteIndex().getName().equals(currentRunWriteIndex.getName())) { | ||
// data stream has not been rolled over in the meantime so record the error against the write index we | ||
// attempted the rollover | ||
errorStore.recordError(currentRunWriteIndex.getName(), e); | ||
errorStore.recordError(clusterService.state().metadata().getProject().id(), currentRunWriteIndex.getName(), e); | ||
} | ||
} | ||
} | ||
|
@@ -1074,7 +1075,7 @@ public void onFailure(Exception e) { | |
if (e instanceof IndexNotFoundException) { | ||
// index was already deleted, treat this as a success | ||
logger.trace("Clearing recorded error for index [{}] because the index was deleted", targetIndex); | ||
errorStore.clearRecordedError(targetIndex); | ||
errorStore.clearRecordedError(clusterService.state().metadata().getProject().id(), targetIndex); | ||
listener.onResponse(null); | ||
return; | ||
} | ||
|
@@ -1157,7 +1158,7 @@ public void onFailure(Exception e) { | |
if (e instanceof IndexNotFoundException) { | ||
// index was already deleted, treat this as a success | ||
logger.trace("Clearing recorded error for index [{}] because the index was deleted", targetIndex); | ||
errorStore.clearRecordedError(targetIndex); | ||
errorStore.clearRecordedError(clusterService.state().metadata().getProject().id(), targetIndex); | ||
listener.onResponse(null); | ||
return; | ||
} | ||
|
@@ -1193,7 +1194,7 @@ public void onFailure(Exception e) { | |
if (e instanceof IndexNotFoundException) { | ||
logger.trace("Data stream lifecycle did not delete index [{}] as it was already deleted", targetIndex); | ||
// index was already deleted, treat this as a success | ||
errorStore.clearRecordedError(targetIndex); | ||
errorStore.clearRecordedError(clusterService.state().metadata().getProject().id(), targetIndex); | ||
listener.onResponse(null); | ||
return; | ||
} | ||
|
@@ -1341,7 +1342,9 @@ static class ErrorRecordingActionListener implements ActionListener<Void> { | |
@Override | ||
public void onResponse(Void unused) { | ||
logger.trace("Clearing recorded error for index [{}] because the [{}] action was successful", targetIndex, actionName); | ||
errorStore.clearRecordedError(targetIndex); | ||
@FixForMultiProject(description = "Don't use default project ID") | ||
final var projectId = Metadata.DEFAULT_PROJECT_ID; | ||
Comment on lines
+1345
to
+1346
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not for this PR. But I wonder whether we could remove @Deprecated(forRemoval=true)
public static ProjectId defaultProjectId() {
return ProjectId.DEFAULT;
} We can then use the above method in all places where default project id is meant to be a placeholder. And use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a great idea, we should have done that earlier :) |
||
errorStore.clearRecordedError(projectId, targetIndex); | ||
} | ||
|
||
@Override | ||
|
@@ -1364,8 +1367,10 @@ static void recordAndLogError( | |
String logMessage, | ||
int signallingErrorRetryThreshold | ||
) { | ||
ErrorEntry previousError = errorStore.recordError(targetIndex, e); | ||
ErrorEntry currentError = errorStore.getError(targetIndex); | ||
@FixForMultiProject(description = "Don't use default project ID") | ||
final var projectId = Metadata.DEFAULT_PROJECT_ID; | ||
ErrorEntry previousError = errorStore.recordError(projectId, targetIndex, e); | ||
ErrorEntry currentError = errorStore.getError(projectId, targetIndex); | ||
if (previousError == null || (currentError != null && previousError.error().equals(currentError.error()) == false)) { | ||
logger.error(logMessage, e); | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
to keep deprecation in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already have local changes ready to make
DataStreamLifecycleService
project aware - overriding these changes. So I'll leave this as is for now.