Skip to content

DPL: Extending ExternalFairMQDeviceProxy benchmark#7729

Merged
matthiasrichter merged 1 commit intoAliceO2Group:devfrom
matthiasrichter:dev-benchmark-proxy
Nov 26, 2021
Merged

DPL: Extending ExternalFairMQDeviceProxy benchmark#7729
matthiasrichter merged 1 commit intoAliceO2Group:devfrom
matthiasrichter:dev-benchmark-proxy

Conversation

@matthiasrichter
Copy link
Copy Markdown
Collaborator

  • handle exception from shm allocation

Depending on the configuration, there might not be enough shared memory,
handling this exception gracefully.

  • more proxy bypassing options

Option '--bypass-proxies' none, all(a), output(o)

We can now bypass both proxies to get the metrics for the pure producer
to checker traffic, or bypass output proxy only to benchmark with one
proxy in between. It is not possible to have only the output proxy.

@matthiasrichter matthiasrichter requested a review from a team as a code owner November 24, 2021 13:36
@ktf
Copy link
Copy Markdown
Member

ktf commented Nov 25, 2021

This looks good. Could you please merge the two automatically opened PRs you have for it and then merge this one?

ktf
ktf previously approved these changes Nov 25, 2021
- handle exception from  shm allocation

Depending on the configuration, there might not be enough shared memory,
handling this exception gracefully.

- more proxy bypassing options

Option '--bypass-proxies' none, all(a), output(o)

We can now bypass both proxies to get the metrics for the pure producer
to checker traffic, or bypass output proxy only to benchmark with one
proxy in between. It is not possible to have only the output proxy.
@matthiasrichter
Copy link
Copy Markdown
Collaborator Author

nice tool with the automatic PRs, but naturally doesn't work with two conflicting changes, in this case I had to do it manually.

And can "squash and commit" be selected by default?

@ktf
Copy link
Copy Markdown
Member

ktf commented Nov 25, 2021

If you merged one, then the second one would have been updated at some point.

I will check if it's possible to make "squash and merge" default.

@ktf
Copy link
Copy Markdown
Member

ktf commented Nov 25, 2021

Apparently it's not possible...

@ktf ktf closed this Nov 25, 2021
@matthiasrichter
Copy link
Copy Markdown
Collaborator Author

@ktf I would add the reference to the text of the PR not only the title, and the project needs to be in the reference I think. So that GitHub is adding cross links to the original PR. This will make the suggestions more visible. I have noticed the feature only now when you mentioned it.

Something like "This is a follow-up to AliceO2Group/AliceO2#PR ." I'm not exactly sure about the syntax

I reopened, think you have closed accidentally.

@ktf
Copy link
Copy Markdown
Member

ktf commented Nov 25, 2021

I think I already updated the script in a way which is close to your suggestion. See:

dc0dd95

you might have to rebase in order to see the new message...

@matthiasrichter
Copy link
Copy Markdown
Collaborator Author

very good!

@matthiasrichter matthiasrichter merged commit 9021bc2 into AliceO2Group:dev Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants