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

global: receive and store job caching info #104

Merged
merged 4 commits into from
Jul 13, 2018

Conversation

dinosk
Copy link
Member

@dinosk dinosk commented Jul 1, 2018

@dinosk dinosk self-assigned this Jul 1, 2018
@dinosk dinosk force-pushed the job-caching branch 2 times, most recently from 6b0baf2 to 3326bad Compare July 1, 2018 14:36
def _update_job_cache(msg):
"""Update caching information for finished job."""
cmd = msg['caching_info']['job_spec']['cmd']
clean_cmd = cmd.split(';')[1]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a "hack" to clean the commnands from r-w-e-serial, that have prepended: cd <path-to-workspace> which includes a unique id, making the command hash different each time.

Copy link
Member

Choose a reason for hiding this comment

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

👍 we can add a comment so we remember

job_id=msg['caching_info'].get('job_id'),
parameters=input_hash,
result_path=msg['caching_info'].get('result_path'),
workspace_hash=calculate_hash_of_dir(
Copy link
Member

Choose a reason for hiding this comment

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

As discussed IRL, we should not store the job cache if workspace_hash failed.

msg['caching_info']['workflow_json'])
directory_hash = calculate_hash_of_dir(
msg['caching_info'].get('workflow_workspace').
replace('data', 'reana/default'))
Copy link
Member

Choose a reason for hiding this comment

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

This is going to invalidate the cache for all non default organization jobs. Something like reanahub/reana-job-controller@a2a73e6 needs to be done. You can get the experiment/organization name from the job_spec, we have to be careful though since soon we will be changing experiment variable name to organization.

msg['caching_info']['job_spec']['cmd'] = clean_cmd
input_hash = calculate_job_input_hash(msg['caching_info']['job_spec'],
msg['caching_info']['workflow_json'])
directory_hash = calculate_hash_of_dir(
Copy link
Member

Choose a reason for hiding this comment

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

As spotted today, RJC controller calculates the hash before the job has been started (see here) but for steps that do thinks like - echo 'Great it is Friday' > test.txt which are fast to execute, the workflow workspace will change by the moment this line is executed, calculating a different workspace_hash

return
cmd = msg['caching_info']['job_spec']['cmd']
# removes cd to workspace, to be refactored
clean_cmd = cmd.split(';')[1]
Copy link
Member

Choose a reason for hiding this comment

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

shall we ';'.join(test.split(';')[1:]) since most probably there will be more ; inside the command?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point, fixed here and in r-w-e-serial 👍

Signed-off-by: Dinos Kousidis <dinos.kousidis@cern.ch>
Signed-off-by: Dinos Kousidis <dinos.kousidis@cern.ch>
Signed-off-by: Dinos Kousidis <dinos.kousidis@cern.ch>
Signed-off-by: Dinos Kousidis <dinos.kousidis@cern.ch>
@diegodelemos diegodelemos merged commit eea657d into reanahub:master Jul 13, 2018
@tiborsimko tiborsimko added this to Done in v0.3.0 Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v0.3.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants