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

[CCR Monitoring] Only collect stats for specified indices #33646

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Sep 12, 2018

Follow up to #33617. Relates to #30086.

As with all other per-index Monitoring collectors, the CcrStatsCollector should only collect stats for the indices the user wants to monitor. This list is controlled by the xpack.monitoring.collection.indices setting and defaults to all indices.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -72,7 +72,7 @@ protected boolean shouldCollect(final boolean isElectedMaster) {
final ClusterState clusterState) throws Exception {
try (ThreadContext.StoredContext ignore = stashWithOrigin(threadContext, MONITORING_ORIGIN)) {
final CcrStatsAction.StatsRequest request = new CcrStatsAction.StatsRequest();
request.setIndices(Strings.EMPTY_ARRAY);
request.setIndices(getCollectionIndices());
Copy link
Contributor Author

@ycombinator ycombinator Sep 12, 2018

Choose a reason for hiding this comment

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

Additionally, would it make sense to also set the following?

request.setIndicesOptions(IndicesOptions.lenientExpandOpen());

The IndexRecoveryCollector and IndexStatsCollector do this too.

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 that makes sense. Otherwise we may throw an index not found exception if the index patterns the user is interested in does not yet exist.

Copy link
Member

Choose a reason for hiding this comment

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

btw the build fails because of the removal of String.EMPTY_ARRAY its import is now unused.

Copy link
Member

Choose a reason for hiding this comment

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

I agree.

@ycombinator ycombinator requested a review from pickypg September 12, 2018 19:22
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@ycombinator ycombinator merged commit b2f78e5 into elastic:master Sep 12, 2018
@ycombinator ycombinator deleted the x-pack/monitoring/ccr/monitoring-indices branch September 12, 2018 22:56
@jasontedor jasontedor added :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features and removed :Data Management/Monitoring labels Sep 13, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

ycombinator added a commit to ycombinator/elasticsearch that referenced this pull request Sep 18, 2018
)

Follow up to elastic#33617. Relates to elastic#30086.

As with all other per-index Monitoring collectors, the `CcrStatsCollector` should only collect stats for the indices the user wants to monitor. This list is controlled by the `xpack.monitoring.collection.indices` setting and defaults to all indices.
ycombinator added a commit that referenced this pull request Sep 18, 2018
Backport of #33646 to 6.x. Original message:

Follow up to #33617. Relates to #30086.

As with all other per-index Monitoring collectors, the `CcrStatsCollector` should only collect stats for the indices the user wants to monitor. This list is controlled by the `xpack.monitoring.collection.indices` setting and defaults to all indices.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants