EH: CS-849 Allow user / admin to define string based custom fields for job accounting - #85
Merged
Conversation
…> qmaster -> JSONL -> qacct pipeline
Adds UA_svalue to the CULL UA_Type schema alongside the existing UA_value
(double). Presence of UA_svalue is the discriminator between string and
numeric custom usage; standard USAGE_ATTR_* code paths never populate it.
The flatfile spool table gets a matching UA_svalue entry so job spool
round-trips preserve string values through qmaster restart.
New usage_parse_value() helper in source/libs/sgeobj/sge_usage.{cc,h}
classifies raw shepherd usage-file values via the discrimination rule:
1. length >= 2 && matched quote-pair ('...' or "..."): strip, UA_svalue
= interior (empty pair "" yields an empty-string UA_svalue).
2. Full double parse: UA_value = the parsed number.
3. Case-insensitive true / false: UA_value = 1 / 0.
4. Else raw string: UA_svalue = the value as-is (including any stray
leading quote from unmatched or mismatched pairs).
A new CTest module at test/libs/sgeobj/test_sgeobj_usage.cc drives the
helper test-first against every case from origin AE3 (17 assertions
covering quote-pair handling, quoted-numeric strings, bool coercion,
scientific notation, empty string, unmatched / mismatched quotes,
single-character stray quote, embedded quotes with no escape processing,
single-quoted strings, and edge numerics). Wired into the sgeobj
CMakeLists via add_executable + add_test + install.
execd's read_dusage in reaper_execd.cc replaces the numeric-only
add_usage() call with a factory-append: usage_parse_value(name, value)
produces a fully-populated UA_Type element; any earlier entry with the
same name is removed; the parsed element is appended to the job report's
usage list.
qmaster's JSONL writer in sge_rusage.cc branches on UA_svalue in the
usage_patterns loop: string values emit as native JSON strings via
writer->String, numerics keep the existing write_json +
reporting_get_double_usage_sum path. In do_accounting_summary=TRUE mode
string elements are silently skipped (DPRINTF-only, not WARNING, to avoid
messages-file flooding on large PE jobs with per-pe_task string custom
usage) because strings have no defined per-pe_task aggregation semantic.
qacct output in ocs_qacct.cc branches on UA_svalue: strings render via
the existing SHOWJOB_STRING_20 formatter, numerics keep SHOWJOB_FLOAT_18_3.
Both share the same 35-char label column so alignment is consistent. The
qacct JSONL reader that builds d->other_usage was previously calling
GetDouble() unconditionally on every custom-usage entry -- extended to
check IsString() / IsNumber() and populate UA_svalue / UA_value
accordingly. Without this fix qacct would misbehave on any acct record
containing a string custom usage value.
The usage_patterns section of sge_conf(5) documents the type-derivation
rule (quote / double / bool / raw) and the accounting_summary=TRUE skip
semantic so operators see the behaviour explained.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CULL's pack/unpack wire format (packstr / unpackstr in libs/cull/pack.cc) cannot distinguish an empty string from nullptr: an empty string is packed as a single NUL byte, and unpackstr sees that leading NUL and returns nullptr. As a result an empty svalue set on the execd side becomes nullptr on the qmaster side after GDI transport, the JSONL writer's `if (svalue != nullptr)` check fails, execution falls through to the numeric branch and emits `"note":0`, dbwriter stores that as `juv_dvalue=0`, and the XOR invariant is violated end-to-end (row has juv_svalue=NULL AND juv_dvalue=0 instead of juv_svalue="" alone). Fix: usage_parse_value() now returns nullptr for `""` and `''` so the value is silently skipped at ingress. Consistent end-to-end semantics — no row for empty custom usage anywhere in the pipeline. The CTest T10 case flips from "element with empty svalue" to "usage_parse_value returns nullptr" and stays green. sge_conf(5) gets a one-sentence note in the usage_patterns section explaining the skip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.