-
Notifications
You must be signed in to change notification settings - Fork 38
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
Adding all the rule states in getTapRecallUsage and a bug fix #8741
base: master
Are you sure you want to change the base?
Conversation
Jenkins results:
|
Aside the minor comment above, there is a fundamental problem here.
I am still puzzled that code could find a sensible value for input lock while ignoring OK rules. That needs to be understood. |
src/python/RucioUtils.py
Outdated
@@ -124,5 +124,5 @@ def getTapeRecallUsage(rucioClient=None, account=None): | |||
|
|||
rules = rucioClient.list_replication_rules(filters=filters) | |||
usage = sum(getRuleQuota(rucioClient, rule['id']) for rule in rules\ | |||
if rule['state'] in ['REPLICATING', 'STUCK', 'SUSPENDED']) # in Bytes | |||
if rule['state'] in ['OK', 'REPLICATING', 'STUCK', 'SUSPENDED', 'INJECT', 'WAITING_APPROVAL']) # in 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.
if you want to select all possible states, why not simply remove this line ?
In any case, although we use automatic approve here, rules in WAITING_APPROVAL should not be counted as "using space". I am not sure about rules in INJECTING, i.e. whether it comes before or after APPROVAL.
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 didn't remove the line as i still wanted to test that only one of these 6 states are being used and not any random value. Acc to Hasan, INJECT is the first step and it comes before WAITING_APPROVAL, followed by REPLICATING.
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.
So we should not count INJECT either. Or account separately for "pendind rules". But let's try not to make it too complicated
I will look at it. |
@belforte The use of getTapeRecallUsage was merged by me on 17th Sept in #8697 and the release we are running is that of v3.240904 (4th September). Hence the changes are not yet in production and therefore we have the correct thing from using rucioClient.get_local_account_usage in the 4th Sept version. |
thanks. So may be we stick with get_local_account_usage for crab_input ? Why bother to sum all rules when we have the total with a single call ? |
We did the change to address #8661 as we wanted to filter by activity=AnalysisTapeRecall instead of user, and use user argument only if provided. |
thanks for fixing my confusion ! and sorry for noise. |
Jenkins results:
|
…etTapeRecallUsage
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Fix #8736