Skip to content

Conversation

martijnvg
Copy link
Member

When a write op with op_type not equal to create targets a data streams then the following error is returned: no such index [null]. This isn't a meaningful error message. Also the the used exception (IndexNotFoundException) will result in a 404 response code.

This PR adds exception handling logic, so that a IllegalArgumentException is returned with a better error message.
This exception will return a 400 response code instead.

Closes #60581

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Data streams)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Aug 6, 2020
Copy link
Contributor

@probakowski probakowski left a comment

Choose a reason for hiding this comment

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

Thanks @martijnvg for fixing this, one small nit, otherwise LGTM

infe.setResources("index_expression", indexExpressions);
if (excludedDataStreams) {
// Allows callers to handle IndexNotFoundException differently based on whether data streams were excluded.
infe.addMetadata("es.excluded_ds", "true");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe refactor es.excluded_ds to named constant?

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

LGTM. Left an optional comment about the error message wording and a question just for my own benefit.

request.concreteIndex(indexNameExpressionResolver.concreteWriteIndex(clusterState, request).getName());
} catch (IndexNotFoundException e) {
if (request.includeDataStreams() == false && e.getMetadataKeys().contains("es.excluded_ds")) {
throw new IllegalArgumentException("only write ops with op_type=create are allowed in data streams");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new IllegalArgumentException("only write ops with op_type=create are allowed in data streams");
throw new IllegalArgumentException("only write ops with an op_type of create are allowed in data streams");

concreteIndex = indexNameExpressionResolver.concreteWriteIndex(state, request.indicesOptions(),
request.indices()[0], false, includeDataStreams);
} catch (IndexNotFoundException e) {
if (includeDataStreams == false && e.getMetadataKeys().contains("es.excluded_ds")) {
Copy link
Contributor

@danhermann danhermann Aug 6, 2020

Choose a reason for hiding this comment

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

Just a question -- I don't understand why includeDataStreams == false is here. I would have thought it would be the opposite if it were addressing the case of indexing into data streams though it does seem to work according to the tests you added.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it is needed (because es.excluded_ds is only set if includeDataStreams is false), but I like to keep it here to highlight that this check is only needed for op types other than create. If includeDataStreams is false then request.opType() is not CREATE.

@martijnvg martijnvg merged commit f3b305a into elastic:master Aug 10, 2020
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Aug 10, 2020
martijnvg added a commit that referenced this pull request Aug 10, 2020
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Data streams Data streams and their lifecycles >non-issue Team:Data Management Meta label for data/management team v7.9.0 v7.10.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"no such index [null]" when indexing into data stream with op_type=index

5 participants