Skip to content

Add user-defined cluster metadata#33325

Merged
AthenaEryma merged 3 commits intoelastic:masterfrom
AthenaEryma:user-cluster-metadata
Sep 4, 2018
Merged

Add user-defined cluster metadata#33325
AthenaEryma merged 3 commits intoelastic:masterfrom
AthenaEryma:user-cluster-metadata

Conversation

@AthenaEryma
Copy link
Copy Markdown
Contributor

Adds a place for users to store cluster-wide data they wish to associate
with the cluster via the Cluster Settings API. This is strictly for
user-defined data, Elasticsearch makes no other other use of these
settings.

Closes #33220

Adds a place for users to store cluster-wide data they wish to associate
with the cluster via the Cluster Settings API. This is strictly for
user-defined data, Elasticsearch makes no other other use of these
settings.
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

Copy link
Copy Markdown
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

Left 2 comments. LGTM otherwise

if (randomBoolean()) {
client().admin().cluster().prepareUpdateSettings().setPersistentSettings(settings).execute().actionGet();
ClusterStateResponse state = client().admin().cluster().prepareState().execute().actionGet();
assertEquals(value, state.getState().getMetaData().persistentSettings().get(key));
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.

can you also add a tests that updates a setting

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.

Will do.

String value = randomRealisticUnicodeOfCodepointLengthBetween(5, 50);

// Make sure we have both key and value in case this fails
logger.info("Attempting to store [{}]: [{}]", key, value);
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.

this logging statement is unnecessary IMO.

any key prefixed with `cluster.metadata.`. For example, to store the email
address of the administrator of a cluster under the key `cluster.metadata.administrator`,
issue this request:

Copy link
Copy Markdown
Contributor

@mayya-sharipova mayya-sharipova Sep 4, 2018

Choose a reason for hiding this comment

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

Could be worth mentioning that settings can be both persistent and transient

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 think most (all?) settings can be set either/both persistent or transient - at least, that's how I read the Cluster Settings API docs, so I don't think it's critical to mention that here.

Copy link
Copy Markdown
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.

This LGTM to me as a v1. We might have to consider adding dedicated APIs for viewing these, or surfacing them in existing APIs (for example, /), but let's get this in and see what comes.

@AthenaEryma AthenaEryma merged commit cfd3fa7 into elastic:master Sep 4, 2018
AthenaEryma added a commit that referenced this pull request Sep 5, 2018
Adds a place for users to store cluster-wide data they wish to associate
with the cluster via the Cluster Settings API. This is strictly for
user-defined data, Elasticsearch makes no other other use of these
settings.
@AthenaEryma AthenaEryma deleted the user-cluster-metadata branch December 7, 2018 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants