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

support json payload template #202

Merged
merged 16 commits into from
Sep 19, 2024

Conversation

qzhuyan
Copy link
Contributor

@qzhuyan qzhuyan commented Jan 24, 2023

Not sure if we should merge this.
this branch used a private fork of EMQTT and intro some features that might not that be useful for others.

This branch added new arg --topics_payload some.json, the json file has defined topics so emqtt-bench now supports publish to different topics. This is new {publish, Topic} method which means some CLI args for publishing are not used.

This branch has its value for testing the QUIC multi streams that maps different priority topics onto different streams to mitigate the HOL blocking issue.

this branch deps on: emqx/emqtt#252

@qzhuyan qzhuyan force-pushed the dev/william/json-payload-template branch from dfe1c97 to 3861b79 Compare January 27, 2023 13:50
@qzhuyan qzhuyan marked this pull request as ready for review February 9, 2023 15:57
@qzhuyan qzhuyan force-pushed the dev/william/json-payload-template branch from 13c30a9 to 7ab26de Compare May 16, 2023 08:36
@qzhuyan qzhuyan force-pushed the dev/william/json-payload-template branch from 7ab26de to f2e942e Compare September 12, 2024 08:33
@@ -25,6 +25,7 @@ Profiles = {profiles,[ {escript, []}
, getopt
, gun
, cowlib
, jsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true.

json encoding with large payload is toooo slow. Took me some time to find the slowness is coming from jsx.
In my demo, or test report, I will use eterm encoding, json encoding is reserved for specific customer.

topic_spec.json Outdated
"QoS": 1,
"payload_encoding": "eterm",
"payload": {"foo1" : "bar1", "timestamp": "0", "Data": "10"},
"stream" : 0,
Copy link
Contributor Author

@qzhuyan qzhuyan Sep 12, 2024

Choose a reason for hiding this comment

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

stream 0 is default, signal stream solution used by TCP TCP/TLS and QUIC single stream.
stream >0 is for multi stream features and will only work with QUIC enabled .

"interval_ms": "5000",
"inject_timestamp" : "ms",
"QoS": 1,
"payload_encoding": "eterm",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

could be json but slow for large payload.

src/emqtt_bench.erl Outdated Show resolved Hide resolved
rebar.config Outdated Show resolved Hide resolved
[emqtt_bench,
prometheus,
quantile_estimator,
cowboy
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add jsx too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not necessary, I added it in emqtt_bench.app.src

Copy link
Contributor

Choose a reason for hiding this comment

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

but isn't this to generate the escript, which will need jsx to decode the template files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is working as expected at: https://github.com/qzhuyan/emqtt-bench/releases/tag/0.4.22%2Bcustm1

I think rebar pack the jsx into the escript archive file if it is defined in the app.src.

maybe we should put these (emqtt_bench, prometheus, quantile_estimator, cowboy) to app src as well?

I think escript_incl_apps is to include none dependable apps such like 'redbug'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is working as expected at: https://github.com/qzhuyan/emqtt-bench/releases/tag/0.4.22%2Bcustm1

I think rebar pack the jsx into the escript archive file if it is defined in app.src.

maybe we should put these (emqtt_bench, prometheus, quantile_estimator, cowboy) to app src as well?

Copy link
Contributor

@thalesmg thalesmg Sep 19, 2024

Choose a reason for hiding this comment

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

I see, I thought this setting could be relevant.

maybe we should put these (emqtt_bench, prometheus, quantile_estimator, cowboy) to app src as well?

I think they are (except the first, which is the app itself). Though quicer would need to be conditionally inserted as well?

src/emqtt_bench.erl Outdated Show resolved Hide resolved
src/emqtt_bench.erl Outdated Show resolved Hide resolved
src/emqtt_bench.erl Outdated Show resolved Hide resolved
src/emqtt_bench.erl Outdated Show resolved Hide resolved
[emqtt_bench,
prometheus,
quantile_estimator,
cowboy
Copy link
Contributor

@thalesmg thalesmg Sep 19, 2024

Choose a reason for hiding this comment

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

I see, I thought this setting could be relevant.

maybe we should put these (emqtt_bench, prometheus, quantile_estimator, cowboy) to app src as well?

I think they are (except the first, which is the app itself). Though quicer would need to be conditionally inserted as well?

@qzhuyan qzhuyan force-pushed the dev/william/json-payload-template branch from 072a53d to fb45cab Compare September 19, 2024 16:36
@qzhuyan qzhuyan merged commit af66a6b into emqx:master Sep 19, 2024
4 checks passed
@qzhuyan qzhuyan deleted the dev/william/json-payload-template branch September 19, 2024 17:50
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.

3 participants