-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ML][Data Frame] fix progress measurement for continuous transforms #43838
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][Data Frame] fix progress measurement for continuous transforms #43838
Conversation
|
Pinging @elastic/ml-core |
| protected void onFinish(ActionListener<Void> listener) { | ||
| // reset the page size, so we do not memorize a low page size forever, the pagesize will be re-calculated on start | ||
| pageSize = 0; | ||
| pageSize = pivot.getInitialPageSize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it more logical to re-calc it at start (line 118). It doesn't matter right now but maybe in future the page size calculation depends on a setting or entails some more other data dependent calculation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the comment "the pagesize will be re-calculated on start" is not true anymore after this change
hendrikmuhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
run elasticsearch-ci/1 |
1 similar comment
|
run elasticsearch-ci/1 |
|
@elasticmachine update branch |
|
run elasticsearch-ci/1 |
|
elasticsearch-ci/1 |
|
@elasticmachine update branch |
|
run elasticsearch-ci/packaging-sample |
…lastic#43838) * [ML][Data Frame] fix progress measurement for continuous transforms * Update DataFrameIndexer.java
Progress is now reset on every new checkpoint.
Some refactoring was necessary so that the changed buckets could be determined BEFORE we reset the progress statistics.