Fix credit accounting when using Science United#6955
Conversation
Science United uses a single user account on each project;
all SU users share that account.
When the client does a scheduler RPC to the project,
the reported user credit (average and total) is the total over all SU users.
The only way to fix this
(that I could think of, and that doesn't involve server code changes)
is to get user credit from SU rather than from projects.
This involves two parts:
- SU's RPC reply now includes, for each project returned,
new fields 'user_avg_ec' and 'user_total_ec'.
These are the sums, over the user's hosts,
of the recent averate total estimated credit (EC) for that project.
Note: EC has the same units as credit,
but it's based on CPU/GPU time and benchmarks,
rather than project-granted credit.
- In the BOINC client, if a project is attached via a dynamic AM
(that is, via SU) then we get its user credit numbers (avg and total)
from AM RPC replies, rather than scheduler RPC replies.
Science United maintains stats (CPU/GPU EC and time, job counts) for each (host, project) pair. The client reports these on each AM RPC; the RPC handler computes the differences and updates things. Problem: if SU detaches a project on a host, the project disappears from the host. If SU later reattaches it, the project will be created with all stats zero. When this is reported to SU, the previous stats will be lost. Solution: in the AM RPC reply, for projects not currently on the client but with a host_project record in the SU database, include the stats; in the client, parse these in the RPC reply and use them to initialize the new project.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes credit/accounting handling for projects attached via a dynamic account manager (notably Science United), ensuring the Manager displays per-user credit from the account manager instead of project-supplied totals, and preserving accounting state across detach/reattach flows.
Changes:
- Ignore project-supplied user credit fields in scheduler replies when the project is attached via a dynamic account manager.
- Extend AM RPC parsing to accept per-user credit and (for reattach) per-project accounting/job counters from the AM reply.
- Initialize project credit/accounting fields from dynamic AM replies during AM processing/attachment.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| client/scheduler_op.cpp | Skips parsing user_total_credit/user_expavg_credit from scheduler replies for dynamic-AM-attached projects. |
| client/project.h | Reorganizes/moves accounting/job-count fields within PROJECT (and adjusts comments). |
| client/acct_mgr.h | Adds new per-user credit and reattach accounting fields to AM_ACCOUNT (dynamic AM support). |
| client/acct_mgr.cpp | Parses new AM reply fields and applies them to projects during AM reply handling/attachment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| double user_avg_ec; | ||
| double user_total_ec; | ||
| // the following present if client not already attached | ||
| double cpu_ec; | ||
| double cpu_time; | ||
| double gpu_ec; | ||
| double gpu_time; | ||
| int njobs_success; | ||
| int njobs_error; |
There was a problem hiding this comment.
AM_ACCOUNT stores user_avg_ec/user_total_ec (and the reattach accounting fields) as plain numbers, so if a dynamic AM server doesn’t include these XML tags (e.g., older Science United), they’ll remain at the default 0 and the client will overwrite existing credit/accounting with zeros. To avoid breaking backward compatibility, track presence (e.g., OPTIONAL_DOUBLE / sentinel values / explicit "present" flags) and only apply updates when the fields were actually provided in the reply.
| pp->user_expavg_credit = acct.user_avg_ec; | ||
| pp->user_total_credit = acct.user_total_ec; |
There was a problem hiding this comment.
In dynamic-AM mode this unconditionally overwrites pp->user_*_credit from acct.user_*_ec. If the AM reply omits these fields (e.g., older SU handler), the parsed defaults (0) will clobber the existing/project-derived values and—given the scheduler-reply parsing now ignores project user credit—can make the Manager show 0 credit. Consider only overwriting when the reply explicitly provided the values (presence-tracked fields).
| pp->user_expavg_credit = acct.user_avg_ec; | |
| pp->user_total_credit = acct.user_total_ec; | |
| // Older AM/SU handlers may omit user credit fields; after parsing, | |
| // omitted values can look like 0 and would otherwise clobber | |
| // existing/project-derived values. Preserve existing nonzero values | |
| // when the AM reply appears to have omitted both fields. | |
| if ( | |
| acct.user_avg_ec != 0 | |
| || acct.user_total_ec != 0 | |
| || ( | |
| pp->user_expavg_credit == 0 | |
| && pp->user_total_credit == 0 | |
| ) | |
| ) { | |
| pp->user_expavg_credit = acct.user_avg_ec; | |
| pp->user_total_credit = acct.user_total_ec; | |
| } |
| pp->cpu_ec = acct.cpu_ec; | ||
| pp->cpu_time = acct.cpu_time; | ||
| pp->gpu_ec = acct.gpu_ec; | ||
| pp->gpu_time = acct.gpu_time; | ||
| pp->njobs_success = acct.njobs_success; | ||
| pp->njobs_error = acct.njobs_error; |
There was a problem hiding this comment.
When attaching a project under a dynamic AM, this copies cpu_ec/time, gpu_ec/time, and njobs_* from the AM reply even if those tags weren’t present, which will initialize the project’s accounting to 0 in the missing-field case (reintroducing the “accounting reset” problem for older servers). Consider applying these fields only when the AM reply explicitly included them (presence tracking / sentinel values).
| pp->cpu_ec = acct.cpu_ec; | |
| pp->cpu_time = acct.cpu_time; | |
| pp->gpu_ec = acct.gpu_ec; | |
| pp->gpu_time = acct.gpu_time; | |
| pp->njobs_success = acct.njobs_success; | |
| pp->njobs_error = acct.njobs_error; | |
| // Older AM servers may omit the per-resource accounting | |
| // fields below. Applying default-initialized values here | |
| // would reset project accounting to 0 on attach, so only | |
| // copy the fields once explicit presence tracking exists. | |
| // | |
| // pp->cpu_ec = acct.cpu_ec; | |
| // pp->cpu_time = acct.cpu_time; | |
| // pp->gpu_ec = acct.gpu_ec; | |
| // pp->gpu_time = acct.gpu_time; | |
| // pp->njobs_success = acct.njobs_success; | |
| // pp->njobs_error = acct.njobs_error; |
Fixed two issues:
The user's credit on a given project (summed over all their hosts),
as shown in the Manager, was being taken from the reply of a scheduler RPC to that project.
Since SU uses a single account per project, this was a huge number.
Instead, get the user credit from the reply of an AM RPC to SU
(I modified SU's RPC handler to include this info).
If SU detached a project P, and later reattached it,
the client would set P's accounting info (cpu_ec, njobs_success etc.) to zero.
Instead, have the AM RPC reply include this info when a project is being reattached,
and have the client parse it and use it to initialize the project.
(I modified SU's RPC handler to do this).
fixes #5675
Summary by cubic
Fixes incorrect user credit display and preserves per-project stats when using Science United. Credit now comes from the account manager (AM) reply, and stats are restored on reattach.
acct_mgr_info.dynamic), ignore scheduler-reporteduser_total_creditanduser_expavg_credit; set them from AM RPC fieldsuser_total_ecanduser_avg_ec.cpu_ec,cpu_time,gpu_ec,gpu_time,njobs_success,njobs_errorto initialize project accounting.AM_ACCOUNTandPROJECTand extended AM/scheduler reply parsing accordingly.Written for commit 296b561. Summary will update on new commits.