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

[ML-DataFrame] Remove ID field from data frame indexer stats #44768

Merged
merged 3 commits into from
Jul 25, 2019

Conversation

droberts195
Copy link
Contributor

@droberts195 droberts195 commented Jul 23, 2019

This is a followup to #44350. The indexer stats used to
be persisted standalone, but now are only persisted as
part of a state-and-stats document. During the review
of #44350 it was decided that we'll stick with this
design, so there will never be a need for an indexer
stats object to store its transform ID as it is stored
on the enclosing document. This PR removes the indexer
stats document ID.

This is a followup to elastic#44350. The indexer stats used to
be persisted standalone, but now are only persisted as
part of a state-and-stats document. During the review
of elastic#44350 it was decided that we'll stick with this
design, so there will never be a need for an indexer
stats object to store its transform ID as it is stored
on the enclosing document. This PR removes the indexer
stats document ID.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@droberts195
Copy link
Contributor Author

Jenkins run elasticsearch-ci/2

Copy link
Contributor

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -57,60 +55,46 @@
LENIENT_PARSER.declareLong(constructorArg(), SEARCH_FAILURES);
}

private final String transformId;

/**
* Certain situations call for a default transform ID, e.g. when merging many different transforms for statistics gather.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: these comments aren't true anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the method as well as the comment as after the other changes I don't think it was adding much over calling the default constructor.

@droberts195 droberts195 merged commit c8974aa into elastic:master Jul 25, 2019
@droberts195 droberts195 deleted the remove_indexer_stats_id branch July 25, 2019 10:44
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Jul 25, 2019
Mutes data frame BWC tests prior to backporting elastic#44768
droberts195 added a commit that referenced this pull request Jul 25, 2019
Mutes data frame BWC tests prior to backporting #44768
droberts195 added a commit that referenced this pull request Jul 25, 2019
This is a followup to #44350. The indexer stats used to
be persisted standalone, but now are only persisted as
part of a state-and-stats document. During the review
of #44350 it was decided that we'll stick with this
design, so there will never be a need for an indexer
stats object to store its transform ID as it is stored
on the enclosing document. This PR removes the indexer
stats document ID.

Backport of #44768
droberts195 added a commit that referenced this pull request Jul 25, 2019
This change adjusts the changes of #44768 to account
for the backport to the 7.x branch in #44848.
polyfractal pushed a commit to polyfractal/elasticsearch that referenced this pull request Jul 29, 2019
…#44768)

This is a followup to elastic#44350. The indexer stats used to
be persisted standalone, but now are only persisted as
part of a state-and-stats document. During the review
of elastic#44350 it was decided that we'll stick with this
design, so there will never be a need for an indexer
stats object to store its transform ID as it is stored
on the enclosing document. This PR removes the indexer
stats document ID.
polyfractal pushed a commit to polyfractal/elasticsearch that referenced this pull request Jul 29, 2019
Mutes data frame BWC tests prior to backporting elastic#44768
polyfractal pushed a commit to polyfractal/elasticsearch that referenced this pull request Jul 29, 2019
…c#44852)

This change adjusts the changes of elastic#44768 to account
for the backport to the 7.x branch in elastic#44848.
jkakavas pushed a commit that referenced this pull request Jul 31, 2019
Mutes data frame BWC tests prior to backporting #44768
jkakavas pushed a commit that referenced this pull request Jul 31, 2019
This change adjusts the changes of #44768 to account
for the backport to the 7.x branch in #44848.
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.

None yet

4 participants