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

Add batch support in ZmqProducerStateTable. #803

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

mint570
Copy link
Contributor

@mint570 mint570 commented Jul 7, 2023

The existing batch implementation in ZmqProducerStateTable only calls the individual API repeatedly. This change brings in the real batch implementation:

  1. Add a new method in ZmqProducerStateTable, send(), which can batch both SET and DEL operations in a single call.
  2. Update the BinarySerializer to encode multiple SET & DEL requests in a single message. The new encoding format is the following:
  • db name
  • table name
  • key1
  • number of attrs in key1
  • field
  • value
  • ...
  • key2
  • number of attrs in key2
  • field
  • value
  • ...
  1. Increase ZMQ buffer size from 4MB to 16MB.

@mint570 mint570 requested a review from liuh-80 July 7, 2023 01:07
@mint570 mint570 force-pushed the zmq_bulk_upstream branch 2 times, most recently from 48f65a9 to ce93d49 Compare July 7, 2023 21:05
@mint570
Copy link
Contributor Author

mint570 commented Jul 10, 2023

@qiluo-msft can you take a look as well? Thanks.

@jarias-lfx
Copy link

/easycla

@mint570 mint570 force-pushed the zmq_bulk_upstream branch 2 times, most recently from 4b5e781 to 9bcfa8b Compare July 20, 2023 19:43
@mint570 mint570 force-pushed the zmq_bulk_upstream branch 4 times, most recently from 45667ab to b0a668e Compare September 19, 2023 02:14
Change-Id: Iec83d86c8541598467a7eefaa37cc232326570f4
@liuh-80
Copy link
Contributor

liuh-80 commented Oct 31, 2023

@mint570 , there still 7 UT failed, it's blocking this PR, please fix and ping me if you need approve again.

Change-Id: I6cdf31582814724792e2046a49592be13725a653
@mint570
Copy link
Contributor Author

mint570 commented Oct 31, 2023

@mint570 , there still 7 UT failed, it's blocking this PR, please fix and ping me if you need approve again.

The failed tests are vs pytest. I am not sure if this PR is causing the failure since the vs pytest doesn't use the new ZMQ API.
I re-triggered the test to run and will see.

@mint570 mint570 merged commit a57cf9e into sonic-net:master Nov 1, 2023
14 checks passed
@mint570 mint570 deleted the zmq_bulk_upstream branch November 1, 2023 16:49
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