-
Notifications
You must be signed in to change notification settings - Fork 64
Introduce memory usage estimation mode in data_frame_analyzer #584
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
Conversation
1a97d9c
to
978a6b4
Compare
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 did a quick pass through. Looking pretty good. I know you're working on tests. I've made a couple of suggestions which might have some knock on effects for testing, so thought I'd just go ahead and submit these.
978a6b4
to
c73e021
Compare
…ory estimations but does not perform any analysis
c73e021
to
462023d
Compare
Thanks. I've just sent this PR to regular review and will address your comments shortly. |
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.
Good stuff! This looks good to go. Just a couple of comment typos and a suggestion. Also, I propose we use the usual error handling mechanism for dealing with the case that the runner is null. I'm going to go ahead and approve because I don't think this needs any further reviews.
…than long values (representing bytes).
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
Introduce a mode in which data_frame_analyzer binary only outputs memory usage estimations but does not perform any analysis. This mode will be used by the new endpoint in Java server.
Relates elastic/elasticsearch#44699