distributed-system-analysis / pbench

A benchmarking and performance analysis framework
http://distributed-system-analysis.github.io/pbench/
GNU General Public License v3.0
188 stars 108 forks source link

Convert TOC to use cache map #3555

Closed dbutenhof closed 1 year ago

dbutenhof commented 1 year ago

PBENCH-1192

This has been on my wishlist for a while, but was blocked by not actually having a usable cache. PR #3550 introduces a functioning (if minimal) cache manager, and this PR layers on top of that. The immediate motivation stems from an email exchange regarding Crucible, and the fact that Andrew would like (not surprisingly) to be able to access the contents of an archived tarball. Having TOC code relying on the Pbench-specific run-toc Elasticsearch index is not sustainable.

Note that, despite #3550 introducing a live cache, this PR represents the first actual use of the in-memory cache map, and some adjustments were necessary to make it work outside of the unit test environment.

Only the final commits here represent the TOC changes: most are from the prior PR.

webbnh commented 1 year ago

If you still have the original commits available (like if the hash IDs happen to be in your terminal scrollback), you should consider starting over from your un-re-based branch.

You might have better luck if you squash your branch (particularly the commits which come from the other PR) before rebasing.

dbutenhof commented 1 year ago

If you still have the original commits available (like if the hash IDs happen to be in your terminal scrollback), you should consider starting over from your un-re-based branch.

You might have better luck if you squash your branch (particularly the commits which come from the other PR) before rebasing.

Yeah, in cases like this I would have been better off squashing both before the rebase, I suspect. But aside from git deciding to duplicate some lines without showing them as conflicts, it wasn't terrible and I definitely wouldn't want to start over! Just a few more segments to clean up: and scanning through the diffs on GitHub pointed out a few comments I'd overlooked after the rebase scramble. I should be able to clean this up "quickly"(ish) tomorrow morning...

webbnh commented 1 year ago

And, for future reference, when you rebase a branch of a branch onto main when the base-branch has been merged, you also have the option of specifying the branching-off place (so, you rebase just the last commit or three, instead of trying to rebase the whole branch hoping that Git will drop most of the commits because they are already in the target).

webbnh commented 1 year ago

3 of 4 checks passed

It looks like maybe you merged this before the tests were done, and GitHub permanently captured that information...interesting! 🤔

dbutenhof commented 1 year ago

3 of 4 checks passed

It looks like maybe you merged this before the tests were done, and GitHub permanently captured that information...interesting! 🤔

Yikes -- I hadn't actually meant to do that, and I think I just assumed it was done. 😦

But I don't think it's "captured" -- even though you have to hit another button to open the test details pane, it's still there and the summary will presumably update when the tests are done.

No, OK; even though the "View Details" shows the tests are running, Jenkins shows they're complete ... so I guess it is captured, and that's even weirder...