Skip to content

clear auto-follow errors on deleting pattern #80544

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

Merged
idegtiarenko merged 12 commits into from
Nov 24, 2021
Merged

clear auto-follow errors on deleting pattern #80544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0f42495
clear auto-follow errors on deleting pattern
idegtiarenko Nov 9, 2021
858b8ba
update test to verify other stats are retained
idegtiarenko Nov 9, 2021
a8a8a7b
restructure the test
idegtiarenko Nov 10, 2021
a030ce1
move expiration outside the listener
idegtiarenko Nov 10, 2021
8c6d46e
Merge branch 'master' into 77723_clear_auto_follow_stats_on_delete
idegtiarenko Nov 10, 2021
4768ea5
fix comments
idegtiarenko Nov 11, 2021
0526560
format and remove duplication
idegtiarenko Nov 11, 2021
2f1df28
Merge branch 'master' into 77723_clear_auto_follow_stats_on_delete
idegtiarenko Nov 22, 2021
0a064fe
use struct key
idegtiarenko Nov 22, 2021
8aedab3
fix comments
idegtiarenko Nov 22, 2021
e862a2e
revert tuple change
idegtiarenko Nov 22, 2021
2e33556
Merge branch 'master' into 77723_clear_auto_follow_stats_on_delete
idegtiarenko Nov 24, 2021
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
43 changes: 35 additions & 8 deletions ...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,28 @@ public class AutoFollowCoordinator extends AbstractLifecycleComponent implements

private volatile TimeValue waitForMetadataTimeOut;
private volatile Map<String, AutoFollower> autoFollowers = Collections.emptyMap();
private volatile Set<String> patterns = Set.of();

// The following fields are read and updated under a lock:
private long numberOfSuccessfulIndicesAutoFollowed = 0;
private long numberOfFailedIndicesAutoFollowed = 0;
private long numberOfFailedRemoteClusterStateRequests = 0;
private final LinkedHashMap<String, Tuple<Long, ElasticsearchException>> recentAutoFollowErrors;
private final LinkedHashMap<AutoFollowErrorKey, Tuple<Long, ElasticsearchException>> recentAutoFollowErrors;

private static final class AutoFollowErrorKey {
private final String pattern;
private final String index;

private AutoFollowErrorKey(String pattern, String index) {
this.pattern = Objects.requireNonNull(pattern);
this.index = index;
}

@Override
public String toString() {
return index != null ? pattern + ':' + index : pattern;
}
}

public AutoFollowCoordinator(
final Settings settings,
Expand All @@ -109,7 +125,7 @@ public AutoFollowCoordinator(
this.executor = Objects.requireNonNull(executor);
this.recentAutoFollowErrors = new LinkedHashMap<>() {
@Override
protected boolean removeEldestEntry(final Map.Entry<String, Tuple<Long, ElasticsearchException>> eldest) {
protected boolean removeEldestEntry(final Map.Entry<AutoFollowErrorKey, Tuple<Long, ElasticsearchException>> eldest) {
return size() > MAX_AUTO_FOLLOW_ERRORS;
}
};
Expand Down Expand Up @@ -162,21 +178,31 @@ public synchronized AutoFollowStats getStats() {
}
}

var recentAutoFollowErrorsCopy = new TreeMap<String, Tuple<Long, ElasticsearchException>>();
for (var entry : recentAutoFollowErrors.entrySet()) {
recentAutoFollowErrorsCopy.put(entry.getKey().toString(), entry.getValue());
}

return new AutoFollowStats(
numberOfFailedIndicesAutoFollowed,
numberOfFailedRemoteClusterStateRequests,
numberOfSuccessfulIndicesAutoFollowed,
new TreeMap<>(recentAutoFollowErrors),
recentAutoFollowErrorsCopy,
timesSinceLastAutoFollowPerRemoteCluster
);
}

synchronized void updateStats(List<AutoFollowResult> results) {
// purge stats for removed patterns
var currentPatterns = this.patterns;
recentAutoFollowErrors.keySet().removeIf(key -> currentPatterns.contains(key.pattern) == false);
// add new stats
long newStatsReceivedTimeStamp = absoluteMillisTimeProvider.getAsLong();
for (AutoFollowResult result : results) {
var onlyPatternKey = new AutoFollowErrorKey(result.autoFollowPatternName, null);
if (result.clusterStateFetchException != null) {
recentAutoFollowErrors.put(
result.autoFollowPatternName,
onlyPatternKey,
Tuple.tuple(newStatsReceivedTimeStamp, new ElasticsearchException(result.clusterStateFetchException))
);
numberOfFailedRemoteClusterStateRequests++;
Expand All @@ -188,9 +214,9 @@ synchronized void updateStats(List<AutoFollowResult> results) {
result.clusterStateFetchException
);
} else {
recentAutoFollowErrors.remove(result.autoFollowPatternName);
recentAutoFollowErrors.remove(onlyPatternKey);
for (Map.Entry<Index, Exception> entry : result.autoFollowExecutionResults.entrySet()) {
final String patternAndIndexKey = result.autoFollowPatternName + ":" + entry.getKey().getName();
var patternAndIndexKey = new AutoFollowErrorKey(result.autoFollowPatternName, entry.getKey().getName());
if (entry.getValue() != null) {
numberOfFailedIndicesAutoFollowed++;
recentAutoFollowErrors.put(
Expand All @@ -199,7 +225,7 @@ synchronized void updateStats(List<AutoFollowResult> results) {
);
LOGGER.warn(
new ParameterizedMessage(
"failure occurred while auto following index [{}] for auto follow " + "pattern [{}]",
"failure occurred while auto following index [{}] for auto follow pattern [{}]",
entry.getKey(),
result.autoFollowPatternName
),
Expand All @@ -211,7 +237,6 @@ synchronized void updateStats(List<AutoFollowResult> results) {
}
}
}

}
}

Expand All @@ -227,6 +252,8 @@ void updateAutoFollowers(ClusterState followerClusterState) {
return;
}

this.patterns = Set.copyOf(autoFollowMetadata.getPatterns().keySet());

@henningandersen henningandersen Nov 22, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should set the patterns to the empty list above when autoFollowMetadata == null too (though I think we never go back to null, so mostly for completeness).

@idegtiarenko idegtiarenko Nov 23, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe we are also not clearing/stopping autoFollowers once metadata is null. If this happens for whatever reason then it might result in auto-followers still running but not producing any stats

@henningandersen henningandersen Nov 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good point, perhaps we can simply add an assert before the return above to express the intent here?

final CopyOnWriteHashMap<String, AutoFollower> autoFollowersCopy = CopyOnWriteHashMap.copyOf(this.autoFollowers);
Set<String> newRemoteClusters = autoFollowMetadata.getPatterns()
.values()
Expand Down
37 changes: 14 additions & 23 deletions .../main/java/org/elasticsearch/xpack/ccr/action/TransportDeleteAutoFollowPatternAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,13 @@
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata;
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata.AutoFollowPattern;
import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class TransportDeleteAutoFollowPatternAction extends AcknowledgedTransportMasterNodeAction<DeleteAutoFollowPatternAction.Request> {

@Inject
Expand Down Expand Up @@ -72,28 +68,23 @@ public ClusterState execute(ClusterState currentState) {

static ClusterState innerDelete(DeleteAutoFollowPatternAction.Request request, ClusterState currentState) {
AutoFollowMetadata currentAutoFollowMetadata = currentState.metadata().custom(AutoFollowMetadata.TYPE);
if (currentAutoFollowMetadata == null) {
throw new ResourceNotFoundException("auto-follow pattern [{}] is missing", request.getName());
}
Map<String, AutoFollowPattern> patterns = currentAutoFollowMetadata.getPatterns();
AutoFollowPattern autoFollowPatternToRemove = patterns.get(request.getName());
if (autoFollowPatternToRemove == null) {
if (currentAutoFollowMetadata == null || currentAutoFollowMetadata.getPatterns().get(request.getName()) == null) {
throw new ResourceNotFoundException("auto-follow pattern [{}] is missing", request.getName());
}

final Map<String, AutoFollowPattern> patternsCopy = new HashMap<>(patterns);
final Map<String, List<String>> followedLeaderIndexUUIDSCopy = new HashMap<>(
currentAutoFollowMetadata.getFollowedLeaderIndexUUIDs()
);
final Map<String, Map<String, String>> headers = new HashMap<>(currentAutoFollowMetadata.getHeaders());
patternsCopy.remove(request.getName());
followedLeaderIndexUUIDSCopy.remove(request.getName());
headers.remove(request.getName());
AutoFollowMetadata newAutoFollowMetadata = removePattern(currentAutoFollowMetadata, request.getName());

AutoFollowMetadata newAutoFollowMetadata = new AutoFollowMetadata(patternsCopy, followedLeaderIndexUUIDSCopy, headers);
ClusterState.Builder newState = ClusterState.builder(currentState);
newState.metadata(Metadata.builder(currentState.getMetadata()).putCustom(AutoFollowMetadata.TYPE, newAutoFollowMetadata).build());
return newState.build();
return ClusterState.builder(currentState)
.metadata(Metadata.builder(currentState.getMetadata()).putCustom(AutoFollowMetadata.TYPE, newAutoFollowMetadata))
.build();
}

private static AutoFollowMetadata removePattern(AutoFollowMetadata metadata, String name) {
return new AutoFollowMetadata(
Maps.copyMapWithRemovedEntry(metadata.getPatterns(), name),
Maps.copyMapWithRemovedEntry(metadata.getFollowedLeaderIndexUUIDs(), name),
Maps.copyMapWithRemovedEntry(metadata.getHeaders(), name)
);
}

@Override
Expand Down
Loading