rumqtt icon indicating copy to clipboard operation
rumqtt copied to clipboard

feat: rumqttc async v5 client request batching

Open flxo opened this issue 1 year ago • 9 comments

Process multiple client requests before flushing the network in the async v5 client.

Type of change

New feature (non-breaking change which adds functionality)

Checklist:

  • [X] Formatted with cargo fmt
  • [X] Make an entry to CHANGELOG.md if it's relevant to the users of the library. If it's not relevant mention why.

Fixes:

  • #810
  • #814
  • #815

flxo avatar Mar 19 '24 11:03 flxo

Hello. There's some CI failure I'm unable to reproduce. Furthermore the test idle_connection_triggers_pings_on_timedoesn't use the v5 async client at all. Are the tests known to be flakey?

flxo avatar Mar 19 '24 13:03 flxo

Pull Request Test Coverage Report for Build 8611480676

Details

  • 160 of 242 (66.12%) changed or added relevant lines in 10 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on pr-batch-fan-out at 38.167%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rumqttc/src/proxy.rs 0 1 0.0%
rumqttc/src/lib.rs 9 13 69.23%
rumqttc/src/v5/framed.rs 3 7 42.86%
rumqttc/src/v5/state.rs 0 6 0.0%
rumqttc/src/v5/mod.rs 11 19 57.89%
rumqttc/src/v5/eventloop.rs 79 138 57.25%
<!-- Total: 160 242
Totals Coverage Status
Change from base Build 8590593284: 38.2%
Covered Lines: 6322
Relevant Lines: 16564

💛 - Coveralls

coveralls avatar Mar 19 '24 14:03 coveralls

It would be really helpful if you could base this PR on #825 so that we can move towards simplifying the codebase significantly!

I am thinking of streamlining all the idiosyncratic code in v4/v5 and making it enough DRY to be maintainer friendly, until then we have to keep both codebases aligned as much as possible(it's already a bit of a wildfire 😅 ).

de-sh avatar Mar 20 '24 16:03 de-sh

I'm putting this on hold until #825 is fixed and merged.

flxo avatar Mar 21 '24 12:03 flxo

Implemented the processing of requests and incoming frames in batches for v5. Minor updates here and there. The v4 and v5 clients share a horrible big amount of code. I just updated the v5 version - v4 probably easy to adopt or even better refactor the common parts.

flxo avatar Mar 26 '24 15:03 flxo

Hello. Can I get a comment on this PR? I'd like to incorporate findings before applying the pattern used in v5 in v3. Thanks.

flxo avatar Apr 02 '24 06:04 flxo

Can I get a comment on this PR?

hey, sorry about the delay but i'm currently caught up with other work, so don't have enough bandwidth for this right now!

thanks for understanding!

swanandx avatar Apr 02 '24 06:04 swanandx

@swanandx Let me know if this PR is still relevant for the project. I'm off keyboard for 3 weeks and would abandon the PR afterwards from my side if there's no progress.

Thanks @flxo

flxo avatar May 10 '24 13:05 flxo

Let me know if this PR is still relevant for the project.

@flxo please note that we value your time and contributions a lot but have been facing a landslide of work due to which our ability to maintain the repo has been significantly hampered. We will merge your PR into a feature branch for now, as we need to continue developing test suites and benchmarks to ensure that we are really seeing a good enough impact for the feature to be merged into main, without breaking the wrong things.

Hope you understand!

I'm off keyboard for 3 weeks

Happy holiday!

de-sh avatar May 10 '24 17:05 de-sh