Add repositories metering API by fcofdez · Pull Request #60371 · elastic/elasticsearch

fcofdez · 2020-07-29T10:59:47Z

This pull request adds a new set of APIs that allows tracking the number of requests performed
by the different repositories.

In order to avoid losing data, the repository statistics are archived after the repository is closed for
a configurable retention period repositories.stats.archive.retention_period. The API exposes the
statistics for the active repositories as well as the archived statistics.

The pull request is quite big but most of the code is needed for exposing the new endpoints and
for the integration tests.

fcofdez · 2020-07-31T12:16:34Z

@elasticmachine update branch

elasticmachine · 2020-08-03T12:36:01Z

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

original-brownbear

Thansk @fcofdez tried to get as far as I could for today and left a number of questions/suggestions for now.

server/src/main/java/org/elasticsearch/repositories/RepositoriesStatsArchive.java

server/src/main/java/org/elasticsearch/repositories/RepositoryStatsSnapshot.java

server/src/main/java/org/elasticsearch/repositories/Repository.java

server/src/main/java/org/elasticsearch/repositories/blobstore/MeteredBlobStoreRepository.java

server/src/main/java/org/elasticsearch/repositories/RepositoriesStatsArchive.java

original-brownbear · 2020-08-04T14:53:23Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

            clusterService.addHighPriorityApplier(this);
        }
        this.verifyAction = new VerifyNodeRepositoryAction(transportService, clusterService, this);
+        this.repositoriesStatsArchive = new RepositoriesStatsArchive(REPOSITORIES_STATS_ARCHIVE_RETENTION_PERIOD.get(settings));


Should this be a cluster setting?

I don't have a strong opinion about that, we can move the new settings to the cluster level if we think it would be better.

plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

...ain/java/org/elasticsearch/xpack/repositories/stats/action/RepositoriesNodeStatsRequest.java

server/src/main/java/org/elasticsearch/repositories/RepositoriesStatsArchive.java

clearing

fcofdez · 2020-08-05T08:59:53Z

Thanks for your initial review @original-brownbear ! I've applied most of your suggestions.

original-brownbear

A couple more points (particularly some questions on how the API should look as well)

original-brownbear · 2020-08-05T10:45:04Z

docs/reference/repositories-stats/apis/clear-repositories-stats-archive.asciidoc

@@ -0,0 +1,31 @@
+[[clear-repositories-stats-archive-api]]


I just realized :) Do we actually want to publicly document this API? If so I think we should make it very clear that this API is not supported/stable (regardless of what kind of stability we might offer here).

We can state that this API is not stable and can suffer changes, I think it can be useful for consumers of this API to have the information at hand. But I'm not sure what's our policy here.

For now, we can just put on the "experimental tag", and mark the license as basic.

We can also add a header that this is an API meant to be used by Elastic's commercial offerings, to avoid any confusion.

server/src/main/java/org/elasticsearch/repositories/RepositoryInfo.java

server/src/main/java/org/elasticsearch/repositories/RepositoryStats.java

...java/org/elasticsearch/xpack/repositories/stats/action/TransportRepositoriesStatsAction.java

original-brownbear · 2020-08-05T11:52:50Z

docs/reference/repositories-stats/apis/clear-repositories-stats-archive.asciidoc

+[[clear-repositories-stats-archive-api-request]]
+==== {api-request-title}
+
+`DELETE /_nodes/<node_id>/_repositories_stats`


After I had some time to think about this ... I wonder, why make life for users so hard by adding node_id here. Wouldn't it be much easier if we just allowed for passing a list of emphemeral_id for archived entries instead so that the consuming API can delete after having safely persisted an id avoiding essentially all kinds of possible races and making this much easier to use?
In the implementation we could simply broadcast the delete to all nodes so we don't have to figure out what id lives on what node I guess to keep it simple.

@ywelsch maybe you have an opinion on this as well?

if we think this API would be hard to use, maybe we can limit its scope to a single node? I added it as I needed a way to clear the archive between tests, if there's a simpler way that can simplify this I'm open to it 👍 .

Perhaps we can use a logical timestamp here to say "only clear repositories that I've observed", and use the cluster state version as such a logical timestamp. I think I would prefer that over a list of all IDs for archived entries. The API would still allow the ability to select nodes (in particular allow for _local).

docs/reference/repositories-stats/apis/get-repositories-stats.asciidoc

Allow clearing up to a particular cluster version.

ywelsch

LGTM

fcofdez · 2020-09-07T13:15:19Z

Thanks for the review @ywelsch!
@original-brownbear do you want to take a look to the latest changes?

original-brownbear

LGTM thanks Francisco :)

fcofdez · 2020-09-07T15:27:09Z

@elasticmachine update branch

fcofdez · 2020-09-08T07:48:00Z

@elasticmachine update branch

This pull request adds a new set of APIs that allows tracking the number of requests performed by the different registered repositories. In order to avoid losing data, the repository statistics are archived after the repository is closed for a configurable retention period `repositories.stats.archive.retention_period`. The API exposes the statistics for the active repositories as well as the modified/closed repositories. Backport of elastic#60371

This pull request adds a new set of APIs that allows tracking the number of requests performed by the different registered repositories. In order to avoid losing data, the repository statistics are archived after the repository is closed for a configurable retention period `repositories.stats.archive.retention_period`. The API exposes the statistics for the active repositories as well as the modified/closed repositories. Backport of #60371

A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in elastic#60371

* Add missing repositories API REST specs A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in #60371 * Move this under the nodes namespace * Rename max_version_to_clear parameter * Update docs URL to use current

* Add missing repositories API REST specs A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in elastic#60371 * Move this under the nodes namespace * Rename max_version_to_clear parameter * Update docs URL to use current

* Add missing repositories API REST specs A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in #60371 * Move this under the nodes namespace * Rename max_version_to_clear parameter * Update docs URL to use current Co-authored-by: Steve Gordon <sgordon@hotmail.co.uk>

* Add missing repositories API REST specs A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in elastic#60371 * Move this under the nodes namespace * Rename max_version_to_clear parameter * Update docs URL to use current

fcofdez force-pushed the metering-api-new branch from ae14671 to bfd1ff2 Compare July 29, 2020 12:52

fcofdez added 2 commits August 2, 2020 13:30

Add repositories stats tracking API

3e77c52

Merge remote-tracking branch 'origin/master' into metering

fc7ca20

fcofdez force-pushed the metering-api-new branch from 0beac90 to fc7ca20 Compare August 2, 2020 11:32

fcofdez added 3 commits August 3, 2020 12:29

Add docs

528da9a

Merge remote-tracking branch 'origin/master' into metering

fcc15c8

Fix build scripts

f71fabb

fcofdez changed the title ~~Add repositories stats tracking API~~ Add repositories utilization statistics API Aug 3, 2020

fcofdez added Team:Distributed Meta label for distributed team. :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement labels Aug 3, 2020

fcofdez marked this pull request as ready for review August 3, 2020 12:36

fcofdez requested review from original-brownbear and ywelsch August 3, 2020 12:36

original-brownbear reviewed Aug 4, 2020

View reviewed changes

fcofdez added 4 commits August 4, 2020 21:37

Address first round of review comments

767edd6

Merge remote-tracking branch 'origin/master' into metering-api-new

20a1dfe

Simplify TransportActions and return repositories stats during archive

02c5d93

clearing

Merge remote-tracking branch 'origin/master' into metering-api-new

a3ce634

fcofdez requested a review from original-brownbear August 5, 2020 09:02

original-brownbear reviewed Aug 5, 2020

View reviewed changes

ywelsch reviewed Aug 10, 2020

View reviewed changes

docs/reference/repositories-stats/apis/get-repositories-stats.asciidoc Outdated Show resolved Hide resolved

fcofdez added 5 commits August 10, 2020 14:10

Merge remote-tracking branch 'origin/master' into metering-api-new

158ad06

Keep track of cluster version on RepositoryStatsSnapshot.

b4bb846

Allow clearing up to a particular cluster version.

Merge remote-tracking branch 'origin/master' into metering-api-new

81b2bf7

Fix test

d0a5995

Merge remote-tracking branch 'origin/master' into metering-api-new

278eaa7

ywelsch approved these changes Sep 7, 2020

View reviewed changes

original-brownbear approved these changes Sep 7, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into metering-api-new

b97f5f3

Merge branch 'master' into metering-api-new

6df2755

Merge branch 'master' into metering-api-new

3e2752b

fcofdez merged commit f55b204 into elastic:master Sep 8, 2020

fcofdez added v7.10.0 backport pending v8.0.0 labels Sep 8, 2020

fcofdez mentioned this pull request Sep 8, 2020

Add repositories metering API 7.x #62088

Merged

fcofdez removed the backport pending label Sep 8, 2020

Mpdreamz mentioned this pull request Nov 16, 2020

7.10.1 Meta Ticket elastic/elasticsearch-net#5096

Closed

61 tasks

stevejgordon mentioned this pull request Nov 26, 2020

Add missing X-Pack repositories REST specs #65548

Merged

stevejgordon mentioned this pull request Dec 17, 2020

7.11.0 Meta Ticket elastic/elasticsearch-net#5198

Closed

stevejgordon added a commit to stevejgordon/elasticsearch that referenced this pull request Jul 15, 2021

Add missing repositories API REST specs

00ba10e

A new experimental X-Pack API was added for repositories, but is missing a REST spec. The API was added in elastic#60371

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Conversation

fcofdez commented Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fcofdez commented Jul 31, 2020

Uh oh!

elasticmachine commented Aug 3, 2020

Uh oh!

original-brownbear left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fcofdez commented Aug 5, 2020

Uh oh!

original-brownbear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywelsch Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

fcofdez commented Sep 7, 2020

Uh oh!

original-brownbear left a comment

Choose a reason for hiding this comment

Uh oh!

fcofdez commented Sep 7, 2020

Uh oh!

fcofdez commented Sep 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fcofdez commented Jul 29, 2020 •

edited

Loading

ywelsch Aug 10, 2020 •

edited

Loading