Skip to content

[v6r22] Pilot Submission statistics is counted by SiteDirector based on Accounting System#4205

Merged
atsareg merged 9 commits into
DIRACGrid:rel-v6r22from
yujikato:rel-v6r22-PilotSubmissionAccounting
Aug 21, 2019
Merged

[v6r22] Pilot Submission statistics is counted by SiteDirector based on Accounting System#4205
atsareg merged 9 commits into
DIRACGrid:rel-v6r22from
yujikato:rel-v6r22-PilotSubmissionAccounting

Conversation

@yujikato

@yujikato yujikato commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

BEGINRELEASENOTES

WMS and Accounting
NEW: Statistics of Pilot submission is sent to Accounting System by SiteDirector.

ENDRELEASENOTES

PilotSubmission_1D
PilotSubmission_2D

@zmathe zmathe left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I have a general comment. This is not accounting, I would propose to add it to the MonitoringSystem. Probably, that time when you developed this accounting type, the monitoring did not exist. It is easy to move this to the MonitoringSystem, will not require lot of work, because both systems are almost identical.

@@ -0,0 +1,31 @@
''' Accounting Type for Pilot Submission
'''
__RCSID__ = "(2019-08-06 +0000) Yuji Kato <kato@hepl.phys.nagoya-u.ac.jp>"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please move RCSID before class declaration( line 7 ) and it must be:

__RCSID__ = "$Id$"

Comment thread AccountingSystem/private/Plotters/PilotSubmissionPlotter.py
return retVal
dataDict, granularity = retVal['Value']
self.stripDataField(dataDict, 0)
dataDict, maxValue = self._divideByFactor(dataDict, granularity)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maxValue is not used:

dataDict, _ = self._divideByFactor(dataDict, granularity)

self.stripDataField(dataDict, 0)
dataDict, maxValue = self._divideByFactor(dataDict, granularity)
dataDict = self._fillWithZero(granularity, reportRequest['startTime'], reportRequest['endTime'], dataDict)
baseDataDict, graphDataDict, maxValue, unitName = self._findSuitableRateUnit(dataDict,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maxValue is not used here as well

Comment thread AccountingSystem/private/Plotters/PilotSubmissionPlotter.py
Comment thread AccountingSystem/private/Plotters/PilotSubmissionPlotter.py
Comment thread AccountingSystem/private/Plotters/PilotSubmissionPlotter.py
Comment thread AccountingSystem/private/Plotters/PilotSubmissionPlotter.py
Comment thread WorkloadManagementSystem/Agent/SiteDirector.py
Comment thread WorkloadManagementSystem/Agent/SiteDirector.py Outdated
@chaen

chaen commented Aug 8, 2019

Copy link
Copy Markdown
Contributor

@zmathe I think it's fine even if it is using the Accounting system.
This PR targets v6r22 so ES is not yet mandatory (from v7 we said)
And BelleII uses it, so it's a good moment to have it in vanilla DIRAC too.
The migration to Monitoring system is quite straight forward indeed, and I seem to remember we agreed with @yujikato to do it in a second step

@yujikato

yujikato commented Aug 9, 2019

Copy link
Copy Markdown
Contributor Author

@zmathe thank you very much for your comments.
As Belle II has not launched Monitoring System, it is not easy for me to move immediately.
After we have monitoring system, I can also work on it.

I fixed other comments.

@andresailer andresailer changed the title Pilot Submission statistics is counted by SiteDirector based on Accounting System [v6r22] Pilot Submission statistics is counted by SiteDirector based on Accounting System Aug 9, 2019
@fstagni

fstagni commented Aug 12, 2019

Copy link
Copy Markdown
Contributor

Hi @yujikato and thanks for this. Questions:

  • do you use this in production in Belle2 or would this be something new for you too?
  • Did you see any performance degradation? The SiteDirector isn't already the fastest of the agents
  • We have deployed several SiteDirectors in LHCb. Each treat a few sites, but some only treat a few CEs from a certain site. These cases are covered, right?

@fstagni

fstagni commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

Can you add a flag to disable this reporting?

@yujikato

Copy link
Copy Markdown
Contributor Author

Thank you for questions and sorry for my late reply due to summer holiday.

* do you use this in production in Belle2 or would this be something new for you too?

It is now running at the BelleDIRAC certification server but not at production environment.
It will be implemented soon.

* Did you see any performance degradation? The SiteDirector isn't already the fastest of the agents

The commits to the AccountingSystem happens for each Site-CE-Queue combination,
not for each pilot jobs.
So the number of commits is much smaller than that for PilotAccounting
(also, there is no additional query to retrieve information).
In my environment, the execution time does not increase more than a few %.

* We have deployed several SiteDirectors in LHCb. Each treat a few sites, but some only treat a few CEs from a certain site. These cases are covered, right?

Is it done by "CEs" option? If so, yes, these case can be is covered.

@yujikato

Copy link
Copy Markdown
Contributor Author

Can you add a flag to disable this reporting?

It is done.

@fstagni

fstagni commented Aug 19, 2019

Copy link
Copy Markdown
Contributor

This one looks OK for merging, but I agree with @zmathe that eventually it should be moved from Accounting to Monitoring. I created a task for that: #4226

@fstagni

fstagni commented Aug 20, 2019

Copy link
Copy Markdown
Contributor

@zmathe you have "requested changes", would you review OK now?

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.

5 participants