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

Core & Internals: rework tombstone handling. Closes #4491, #4436 and #4188 #4594

Merged
merged 1 commit into from Jun 15, 2021

Conversation

rcarpa
Copy link
Contributor

@rcarpa rcarpa commented Apr 30, 2021

Allow setting a default tombstone on any replica creation. Introduce two
rse attributes which allow to customize the behavior per rse:

  • One allows to define the normal replica creation tombstone
    (defaults to 0: "no tombstone")
  • The other used to configure the multihop temporary replica tombstone
    (defaults to 2 hours in the future)
    Use a negative value to set the epoch tombstone. Or 0 to explicitly
    set a null tombstone.

Protect replicas which are used as sources from deletion even if their
tombstone is set to epoch. Add an integration test which verifies this
behavior.
Add a submitter test ensuring that multihop sources are created;
and that the temp multihop replicas are created with a tombstone.

Remove the redundant behavior which touches the tombstone in the
finisher. The 2 remaining protections for intermediate multihop replicas
are now: 1) entries in the source table; 2) the default multihop
tombstone delay of 2 hours into the future.

@rcarpa
Copy link
Contributor Author

rcarpa commented Apr 30, 2021

The first commit was submitted as a separate PR (#4577)

@rcarpa rcarpa force-pushed the patch-4491-default_tombstone_replicas branch 3 times, most recently from b2a1367 to 8ea8d13 Compare May 6, 2021 09:07
Copy link
Contributor

@mlassnig mlassnig left a comment

Choose a reason for hiding this comment

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

looks good from a code point of view. @bari12 can you check for semantics as well pls.

bziemons
bziemons previously approved these changes May 19, 2021
Copy link
Member

@bziemons bziemons left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@bari12 bari12 left a comment

Choose a reason for hiding this comment

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

I think there is a misconception here 😄

The default_tombstone should only be applied for RSEs which have the tombstone_delay set. For all the other RSEs, current behaviour should be kept. As far as as I can see here, we would put the 2 week tombstone on everything which does not have a tombstone_delay set.

For the intermediate replicas of multihops I think we need to do it a bit different. There we should have a global (config table) variable which specifies the tombstone delay.

@rcarpa
Copy link
Contributor Author

rcarpa commented May 19, 2021

I updated thing to not set any default tombstone on normal replica creation. And to set it two hours into the future for the multihop tombstone. I'll work on a separate commit now to make the multihop_tombstone_delay also available as a config variable, which will be used for RSEs which don't have it set per-rse.

@rcarpa rcarpa force-pushed the patch-4491-default_tombstone_replicas branch 2 times, most recently from 2d16ac5 to 41a9fa3 Compare May 25, 2021 14:51
…#4436 and rucio#4188

Allow setting a default tombstone on any replica creation. Introduce two
rse attributes which allow to customize the behavior per rse:
- One allows to define the normal replica creation tombstone
(defaults to 0: "no tombstone")
- The other used to configure the multihop temporary replica tombstone
(defaults to 2 hours in the future)
Use a negative value to set the epoch tombstone. Or 0 to explicitly
set a null tombstone.

Protect replicas which are used as sources from deletion even if their
tombstone is set to epoch. Add an integration test which verifies this
behavior.
Add a submitter test ensuring that multihop sources are created;
and that the temp multihop replicas are created with a tombstone.

Remove the redundant behavior which touches the tombstone in the
finisher. The 2 remaining protections for intermediate multihop replicas
are now: 1) entries in the source table; 2) the default multihop
tombstone delay of 2 hours into the future.

introduce multihop_transfer_delay configuration:
To allow setting a default value for all RSEs used in multihops.
@rcarpa rcarpa force-pushed the patch-4491-default_tombstone_replicas branch from 41a9fa3 to f38bfe3 Compare June 15, 2021 14:04
Copy link
Member

@bari12 bari12 left a comment

Choose a reason for hiding this comment

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

Note: for the more strict replica deletion selection (which filters out sources) we have to see if this affects deletion too negatively. Perhaps we have to switch here to only exclude sources we are actually using.

@bari12
Copy link
Member

bari12 commented Jun 15, 2021

Merging as a feature (For 1.26)

@bari12 bari12 merged commit 0a8cf93 into rucio:master Jun 15, 2021
@rcarpa rcarpa deleted the patch-4491-default_tombstone_replicas branch September 13, 2021 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants