Skip to content

Conversation

@portante
Copy link
Member

@portante portante commented May 11, 2022

The pbench-send-tools command must use the directory that was used for starting/stopping tools.

@portante portante added bug Agent specjbb2005 Related to the specjbb2005 benchmark script. Backport PRs with this label should be considered for back porting to previous release branches labels May 11, 2022
@portante portante added this to the v0.72 milestone May 11, 2022
@portante portante self-assigned this May 11, 2022
webbnh
webbnh previously approved these changes May 11, 2022
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

I suspect that there are some subtleties here that I'm not appreciating, but I guess I have no problems with this change.

@portante
Copy link
Member Author

I suspect that there are some subtleties here that I'm not appreciating

The Tool Meister sub-system requires that the same target directory for start, stop, and send be used.

For SpecJBB2005, we use tool triggers, where the target directories for the start and stop operations are generated by pbench-tool-trigger. That script uses a simple monotonically incrementing number in an iteration directory, %d-default, where it appends the "sample" directory with the requested sample number [1]. For pbench-specjbb2005, it is always 1.

This means that the start and stop operations issued by pbench-tool-trigger use one directory name, and pbench-specjbb2005 invokes the send operation using a "renamed" directory.

This fix moves the pbench-send-tools call up and changes the directory name to be the expected %d-default/sample1.

@webbnh
Copy link
Member

webbnh commented May 11, 2022

The Tool Meister sub-system requires that the same target directory for start, stop, and send be used.

I figured that out from context. 😉 Do we describe the reason for that somewhere?

This means that the start and stop operations issued by pbench-tool-trigger use one directory name, and pbench-specjbb2005 invokes the send operation using a "renamed" directory.

This fix moves the pbench-send-tools call up and changes the directory name to be the expected %d-default/sample1.

Right...so what I'm wondering about is, since we have three things to do -- invoke pbench-send-tools, rename the directory, and run the postprocessing -- and we cannot do the rename first, is there any advantage to doing it second instead of last? (Doing it last makes better logical sense to me, but perhaps there is a different reason why we can't....)

@portante
Copy link
Member Author

The Tool Meister sub-system requires that the same target directory for start, stop, and send be used.

I figured that out from context. 😉 Do we describe the reason for that somewhere?

https://github.com/distributed-system-analysis/pbench/blob/main/lib/pbench/agent/tool_meister.py#L1533

The directory is the "identity" of what was collected so that it can be requested to send. If you change the directory, that is a different set of data to send. The tool meister sub-system is not told a "rename" happened, and has no facility for that (nor would we want one since that would like concerns of the client into the tool meister sub-system).

This means that the start and stop operations issued by pbench-tool-trigger use one directory name, and pbench-specjbb2005 invokes the send operation using a "renamed" directory.
This fix moves the pbench-send-tools call up and changes the directory name to be the expected %d-default/sample1.

Right...so what I'm wondering about is, since we have three things to do -- invoke pbench-send-tools, rename the directory, and run the postprocessing -- and we cannot do the rename first, is there any advantage to doing it second instead of last? (Doing it last makes better logical sense to me, but perhaps there is a different reason why we can't....)

I am not sure about that. I don't know, off-hand, if the post-processed tool results need to work off of the final name or is independent.

I know that we don't want to record the generic tool trigger iteration names but want to record what pbench-specjbb2005 uses for iteration names.

We know the send has to happen before the rename, so why do we need to consider changing the rest of the existing logic?

@webbnh
Copy link
Member

webbnh commented May 11, 2022

I don't know, off-hand, if the post-processed tool results need to work off of the final name or is independent.

I don't either, which is why I was gently inquiring.... 😁

I know that we don't want to record the generic tool trigger iteration names

I was concerned about that, as well.

why do we need to consider changing the rest of the existing logic?

"Continuous improvement?" 😇

The `pbench-send-tools` command must use the directory that was used
for starting/stopping tools.
Copy link
Member Author

@portante portante left a comment

Choose a reason for hiding this comment

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

Ready for review.

@portante portante marked this pull request as ready for review May 17, 2022 19:16
@portante portante requested a review from webbnh May 17, 2022 19:16
@ndokos ndokos self-requested a review May 17, 2022 19:19
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Still looks good.

@portante portante merged commit fbcd7f6 into distributed-system-analysis:main May 17, 2022
portante added a commit to portante/pbench that referenced this pull request May 17, 2022
This is a back-port of commit fbcd7f6 (PR distributed-system-analysis#2831) from `main`.

The `pbench-send-tools` command must use the directory that was used
for starting/stopping tools.
portante added a commit that referenced this pull request May 17, 2022
This is a back-port of commit fbcd7f6 (PR #2831) from `main`.

The `pbench-send-tools` command must use the directory that was used
for starting/stopping tools.
@portante portante removed the Backport PRs with this label should be considered for back porting to previous release branches label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent bug specjbb2005 Related to the specjbb2005 benchmark script.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants