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

The Submitter should skip requests from expired rules #6505

Closed
dchristidis opened this issue Feb 21, 2024 · 0 comments · Fixed by #6583
Closed

The Submitter should skip requests from expired rules #6505

dchristidis opened this issue Feb 21, 2024 · 0 comments · Fixed by #6583
Assignees
Milestone

Comments

@dchristidis
Copy link
Contributor

dchristidis commented Feb 21, 2024

Description

When the Cleaner handles an expired rule, it deletes requests in state QUEUED (and states prior to it) and it tries to cancel (to FTS) requests in state SUBMITTED. The Submitter, however, tries to submit QUEUED requests. This can create two problems: (1) contention between the two daemons and (2) inefficiency (submitting a request only to cancel it shortly after).

Motivation

Improve overall Rucio performance.

Change

Adjust the query in core.request.list_and_mark_transfer_requests_and_source_replicas() to skip requests on expired (and soon-to-expire) rules.

There’s two cases that we need to be aware of:

  1. Requests without any rule attached to them (e.g. multi-hop transfers, replica recovery)
  2. Request re-use (if there’s two or more rules that result in a transfer of the same file to the same destination, then the first request to be created by one rule is reused by the others)
@dchristidis dchristidis self-assigned this Feb 21, 2024
voetberg added a commit to voetberg/rucio that referenced this issue Mar 8, 2024
rucio#6505

Replaces throwing an assert error with raising a warning.
voetberg added a commit to voetberg/rucio that referenced this issue Mar 8, 2024
rucio#6505

Replaces throwing an assert error with raising a warning.
voetberg added a commit to voetberg/rucio that referenced this issue Mar 8, 2024
rucio#6505

Replaces throwing an assert error with raising a warning.
voetberg added a commit to voetberg/rucio that referenced this issue Mar 8, 2024
rucio#6505

Replaces throwing an assert error with raising a warning.
voetberg added a commit to voetberg/rucio that referenced this issue Mar 8, 2024
rucio#6505

Replaces throwing an assert error with raising a warning.
dchristidis added a commit to dchristidis/rucio that referenced this issue Mar 28, 2024
The use of multiple values may bring into question why a particular
value has been chosen.  It seems preferable to standardise on two
possible values:

* By default, use `None`.  This implies that the rule lifetime has no
  effect on the test.
* When denoting an expired rule, use `-1`.
dchristidis added a commit to dchristidis/rucio that referenced this issue Mar 28, 2024
When the Cleaner handles an expired rule, it deletes requests in state
QUEUED (and states prior to it) and it tries to cancel (to FTS) requests
in state SUBMITTED.  The Submitter, however, tries to submit QUEUED
requests.  This can create two problems: (1) contention between the two
daemons and (2) inefficiency (submitting a request only to cancel it
shortly after).

This commit adjusts the underlying database query to skip requests whose
directly-associated rule has expired.

There are two somewhat special cases that won’t be fully handled:
(1) requests without any rule associated to them (e.g. multi-hop
transfers, replica recovery) and (2) request which are reused (if there
are two or more rules that result in a transfer of the same file to the
same destination, then the first request to be created by one rule is
reused by the others).
rdimaio pushed a commit that referenced this issue Mar 29, 2024
The use of multiple values may bring into question why a particular
value has been chosen.  It seems preferable to standardise on two
possible values:

* By default, use `None`.  This implies that the rule lifetime has no
  effect on the test.
* When denoting an expired rule, use `-1`.
rdimaio pushed a commit that referenced this issue Mar 29, 2024
When the Cleaner handles an expired rule, it deletes requests in state
QUEUED (and states prior to it) and it tries to cancel (to FTS) requests
in state SUBMITTED.  The Submitter, however, tries to submit QUEUED
requests.  This can create two problems: (1) contention between the two
daemons and (2) inefficiency (submitting a request only to cancel it
shortly after).

This commit adjusts the underlying database query to skip requests whose
directly-associated rule has expired.

There are two somewhat special cases that won’t be fully handled:
(1) requests without any rule associated to them (e.g. multi-hop
transfers, replica recovery) and (2) request which are reused (if there
are two or more rules that result in a transfer of the same file to the
same destination, then the first request to be created by one rule is
reused by the others).
@dchristidis dchristidis added this to the 34.1.0 milestone Mar 29, 2024
bari12 pushed a commit that referenced this issue Apr 2, 2024
The use of multiple values may bring into question why a particular
value has been chosen.  It seems preferable to standardise on two
possible values:

* By default, use `None`.  This implies that the rule lifetime has no
  effect on the test.
* When denoting an expired rule, use `-1`.
bari12 pushed a commit that referenced this issue Apr 2, 2024
When the Cleaner handles an expired rule, it deletes requests in state
QUEUED (and states prior to it) and it tries to cancel (to FTS) requests
in state SUBMITTED.  The Submitter, however, tries to submit QUEUED
requests.  This can create two problems: (1) contention between the two
daemons and (2) inefficiency (submitting a request only to cancel it
shortly after).

This commit adjusts the underlying database query to skip requests whose
directly-associated rule has expired.

There are two somewhat special cases that won’t be fully handled:
(1) requests without any rule associated to them (e.g. multi-hop
transfers, replica recovery) and (2) request which are reused (if there
are two or more rules that result in a transfer of the same file to the
same destination, then the first request to be created by one rule is
reused by the others).
voetberg pushed a commit to voetberg/rucio that referenced this issue Apr 15, 2024
The use of multiple values may bring into question why a particular
value has been chosen.  It seems preferable to standardise on two
possible values:

* By default, use `None`.  This implies that the rule lifetime has no
  effect on the test.
* When denoting an expired rule, use `-1`.
voetberg pushed a commit to voetberg/rucio that referenced this issue Apr 15, 2024
When the Cleaner handles an expired rule, it deletes requests in state
QUEUED (and states prior to it) and it tries to cancel (to FTS) requests
in state SUBMITTED.  The Submitter, however, tries to submit QUEUED
requests.  This can create two problems: (1) contention between the two
daemons and (2) inefficiency (submitting a request only to cancel it
shortly after).

This commit adjusts the underlying database query to skip requests whose
directly-associated rule has expired.

There are two somewhat special cases that won’t be fully handled:
(1) requests without any rule associated to them (e.g. multi-hop
transfers, replica recovery) and (2) request which are reused (if there
are two or more rules that result in a transfer of the same file to the
same destination, then the first request to be created by one rule is
reused by the others).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant