Sending empty parameter hashmap in case of retried execution#627
Sending empty parameter hashmap in case of retried execution#627dushyantk1509 wants to merge 5 commits intolinkedin:tuning_20190221from
Conversation
|
|
||
| INSERT INTO flow_definition(id, flow_def_id, flow_def_url) VALUES | ||
| (10003,'https://ltx1-holdemaz01.grid.linkedin.com:8443/manager?project=AzkabanHelloPigTest&flow=countByCountryFlow','https://ltx1-holdemaz01.grid.linkedin.com:8443/manager?project=AzkabanHelloPigTest&flow=countByCountryFlow'); | ||
| (10003,'https://ltx1-holdemaz01.grid.linkedin.com:8443/manager?project=AzkabanHelloPigTest&flow=countByCountryFlow','https://ltx1-holdemaz01.grid.linkedin.com:8443/manager?project=AzkabanHelloPigTest&flow=countByCountryFlow'), |
There was a problem hiding this comment.
Kindly remove internal Url links.
There was a problem hiding this comment.
These lines are already there. I have not modified them except for ';' changed to ',' as I wanted to another line.
There was a problem hiding this comment.
Okay, but you can help with correcting this and if need can ask for the help of the author of the respective part.
| logger.info(" Retry "); | ||
| logger.info(" Retried job execution " + tuningInput.getJobExecId()); | ||
| applyPenalty(tuningInput.getJobExecId()); | ||
| jobSuggestedParamSet = getBestParamSet(tuningInput.getJobDefId()); |
There was a problem hiding this comment.
Will there be no parameters suggested in case of retry?
There was a problem hiding this comment.
No. It sends empty hashmap so that default parameters can be applied for retried execution.
There was a problem hiding this comment.
So was getting bestParameters and sending them was a BUG which you fixed in this PR?
There was a problem hiding this comment.
Yes because after getting these parameters dr. elephant is adding the retried job execution to TuningJobExecutionParamSet against these parameters which is not correct as this is a dummy call. Retried execution always run with default parameters. It ignores the parameters sent by dr. elephant.
Therefore this step was unnecessary step and next step was not correct.
| List<JobSuggestedParamValue> jobSuggestedParamValues = null; | ||
| if (tuningInput.getRetry()) { | ||
| logger.info(" Retry "); | ||
| logger.info(" Retried job execution " + tuningInput.getJobExecId()); |
There was a problem hiding this comment.
Is this method called when execution is re-tried or is about to retry?
There was a problem hiding this comment.
About to retry. Will correct in next commit.
|
@dushyantk1509 Can you mention the problem statement in the Description section of the PR as it will provide more context about the PR. Also the doc link you mentioned above is not accessible for me using my external email-id, can you have a look so that it would be available to all. |
Now you won't need docs. |
* Integrating Spark and MR exception fingerprinting so results for both will be visible on Dr.Elephant's UI * Changing OracleJDK to OpenJDK as travisCI is not supporting OracleJDK8 * Adding IDENTITY generation strategy for ID * Added unit tests for Exception Fingerprinting * Adding test files * Addressing the review comments
2 participants
When a job is retried, it makes a dummy getCurrentRunParameter call to dr. elephant. In this way tuneIN knows that there was a failure with earlier parameters and penalize the parameters accordingly. In current implementation, best parameters are returned as an output of this dummy call even though we don’t apply these parameters and a new entry is made in tuningJobExecutionParamSet which is not correct.
In this change, empty parameter hash map will return as an output of retried dummy getCurrentRunParameter call and make entry in tuningJobExecutionParamSet only for non-tried executions.