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

Issue 81: Customize pravega payload attributes before submitting to make each payload unique #80

Merged
merged 6 commits into from
Aug 3, 2023

Conversation

dada-dell-emc
Copy link
Contributor

@dada-dell-emc dada-dell-emc commented Jul 24, 2023

Change log description
Added changes to customize pravega payload Timestamp attribute before submitting to Pravega, to make each payload unique.

Purpose of the change
Fixes #81.

What the code does
This PR does the following:
Introduced new boolean configuration attribute customPayload.
If customPayload is set to true, then customize json request payload with Timestamp dynamic value in ms.

How to verify it
Enable customPayload configuration for pravega and then run omb for pravega driver with sample payload custom-payload-idrac.data and workload custom-payload-idrac.yaml shared in this repository.
After that when you read those messages in pravega consumer check Timestamp attribute value, you will get different values almost each time in milliseconds. Or you can enble debug log in omb which will print customized payload for pravega, check Timestamp attribute value,it will print different values almost each time in milliseconds.

…lue to enabled or disable customizing payload for pravega

Signed-off-by: dada-dell-emc <[email protected]>
@dada-dell-emc dada-dell-emc requested a review from shshashwat July 26, 2023 14:47
@dada-dell-emc dada-dell-emc changed the title Added Customizing json request payload with Timestamp value Issue 81: Customize pravega payload attributes before submitting to make each payload unique Jul 26, 2023
@ShwethaSNayak
Copy link

Did we verify these changes through any tests?

@dada-dell-emc
Copy link
Contributor Author

Did we verify these changes through any tests?

We verified these changes from logs, there is no test cases for drivers

Copy link

@sachin-j-joshi sachin-j-joshi left a comment

Choose a reason for hiding this comment

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

LGTM

@RaulGracia
Copy link
Contributor

@sachin-j-joshi @dada-dell-emc do you know that the most recent version of the Pravega benchmark is in the official repository? https://github.com/openmessaging/benchmark
I say that because I don't know if the version in this repo actually has the most recent changes compared to the official repo. Besides, we contributed the Pravega driver to the official OpenMessaging repo for greater visibility and wider usage; keeping that repo updated would be great, as anybody benchmarking other system could also try Pravega with the most recent changes.

Copy link
Contributor

@RaulGracia RaulGracia left a comment

Choose a reason for hiding this comment

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

If the intention of this PR is just to make payloads unique on a per-write basis, please consider a more efficient approach, as the current one looks quite expensive for a benchmark. Also, please note my comment about contributing in the future (maybe not this change, but future improvements such as upgrading dependencies) to the official OpenMessaging Benchmark repo, as we contributed the Pravega driver there a couple of years back: https://github.com/openmessaging/benchmark/tree/master/driver-pravega

@dada-dell-emc
Copy link
Contributor Author

dada-dell-emc commented Aug 3, 2023

If the intention of this PR is just to make payloads unique on a per-write basis, please consider a more efficient approach, as the current one looks quite expensive for a benchmark. Also, please note my comment about contributing in the future (maybe not this change, but future improvements such as upgrading dependencies) to the official OpenMessaging Benchmark repo, as we contributed the Pravega driver there a couple of years back: https://github.com/openmessaging/benchmark/tree/master/driver-pravega

These changes are mainly just to submit the payload into pravega and not benchmarking pravega, so that QE can maintain throughput for their end to end testing.
We will improve efficiency of code for benchmark in our next PR.
New issue #82 created to address it.

@sachin-j-joshi sachin-j-joshi merged commit 0817a2a into pravega:master Aug 3, 2023
@sachin-j-joshi
Copy link

@dada-dell-emc I have merged this PR based on 3 approvals. Please work towards addressing issues
in #82

dada-dell-emc added a commit to munish1789/benchmark that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom payload runtime changes for pravega
6 participants