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

resource: round timestamp of drained ranks #5866

Merged
merged 1 commit into from Apr 8, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Apr 7, 2024

This PR rounds the drain timestamp in the resource module while building the drain object for a resource.status response. The idea here is that this will allow more compression in the drain object in the response and also result in less lines of output in flux resource drain (when sub-second precision in the output is used)

A kind of best case scenario is show by separately draining 16000 nodes, such that each rank has a different timestamp (but many are actually within the same second)

before

expecting success:
        flux dmesg -c >/dev/null &&
        time rpc "resource.status" >rstatus.json
        flux dmesg -HL


real    0m0.766s
user    0m0.193s
sys     0m0.040s
ok 7 - time resource.status rpc

expecting success:
        time flux python -m cProfile -o prof.out $flux_resource status

     STATE UP NNODES NODELIST
     avail  ✔      1 fake0
    avail*  ✗    383 fake[16001-16383]
  drained*  ✗  16000 fake[1-16000]

real    0m6.965s
user    0m5.753s
sys     0m0.092s
expecting success:
        flux dmesg -c >/dev/null &&
        time rpc "resource.status" >rstatus.json
        flux dmesg -HL


real    0m0.292s
user    0m0.205s
sys     0m0.031s
ok 7 - time resource.status rpc

expecting success:
        time flux python -m cProfile -o prof.out $flux_resource status

     STATE UP NNODES NODELIST
     avail  ✔      1 fake0
    avail*  ✗    383 fake[16001-16383]
  drained*  ✗  16000 fake[1-16000]

real    0m5.524s
user    0m4.784s
sys     0m0.065s

Note the RPC time was reduced from 0.766s to 0.292s -- probably from massively reducing the payload size.
There's only a slight difference in flux resource status because the command is now bound by performance of librlist operations (mostly rlist_diff).

Problem: Timestamps for drained nodes are kept with sub-microsecond
precision. This causes the drain object in the resource.status response
to be much less compressible with arguably little benefit. It also
reduces the number of lines that can be coalesced in `flux resource
drain`.

Round timestamps while building the drainset object. This reduces
the drain object size when a significant number of ranks are drained
within the same second.
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

codecov bot commented Apr 8, 2024

Codecov Report

Merging #5866 (52e9d0d) into master (42fdaaa) will decrease coverage by 0.03%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5866      +/-   ##
==========================================
- Coverage   83.29%   83.27%   -0.03%     
==========================================
  Files         514      514              
  Lines       82758    82758              
==========================================
- Hits        68937    68920      -17     
- Misses      13821    13838      +17     
Files Coverage Δ
src/modules/resource/drain.c 78.50% <ø> (ø)

... and 10 files with indirect coverage changes

@grondo
Copy link
Contributor Author

grondo commented Apr 8, 2024

Thanks! Setting MWP

@mergify mergify bot merged commit 942c5cd into flux-framework:master Apr 8, 2024
35 checks passed
@grondo grondo deleted the drain-round-timestamp branch April 8, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants