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

[DRAFT] Add correlation identifier to match commands and responses #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mintobit
Copy link

@mintobit mintobit commented Mar 23, 2025

Fixes DiceDB/dice#1591

The source of the issue is absence of synchronization when Client is writing to and reading from TCP connection.

Need your feedback, to make sure you are ok with the approach I took:

  • Use fixed 4 byte header for each message (wire.Command/wire.Response)
  • Add correlation_id to each command/response to match them
  • On Client side, read responses from net.Conn in a loop
  • Put responses in corresponding channel (map[corr_id] = chan *Response)
  • Each Client.Fire() reads from a corresponding channel

Here are the links to all of the PRs (this fix requires changes to multiple repositories):
dicedb-go PR
dice PR
dicedb-protos PR

  • Communicate with maintainers on the overall approach
  • Make code production-ready (error handling etc)

Before this change:
before
After this change:
after

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.

Bug: Concurrent queries over the same connection
1 participant