Skip to content

fix: Handle multiple watch commands for same client and minor improvements #1738

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Kaushal-26
Copy link
Contributor

@Kaushal-26 Kaushal-26 commented May 17, 2025

Running other watch command after killing first watch does not work

Steps to reproduce:

  • Kill watch connection between client and server using CTRL+C and restart another does not work properly

Example with dicedb-cli run on 2 terminals:

After changes:

>> make run
go run main.go
localhost:7379> SET a v1
OK
localhost:7379> SET k w1
OK
localhost:7379> GET.WATCH k
entered the watch mode for GET.WATCH k
OK [fingerprint=6645377565653310095] "w2" # SET k w2 (on Terminal 2)
OK [fingerprint=6645377565653310095] "w3" # SET k w3 (on Terminal 2)
^Cexiting the watch mode. back to command mode
localhost:7379> GET a
OK "v1"
localhost:7379> GET k
OK "w3"
localhost:7379> GET.WATCH k
entered the watch mode for GET.WATCH k
OK [fingerprint=6645377565653310095] "w3" # GET k (on Terminal 2)
OK [fingerprint=6645377565653310095] "e1" # SET k e1 (on Terminal 2)
OK [fingerprint=6645377565653310095] "e2" # SET k e2 (on Terminal 2)
^Cexiting the watch mode. back to command mode
localhost:7379> GET a
OK "v1"
localhost:7379> GET k
OK "e2"

Before changes:

>> make run
go run main.go
localhost:7379> SET a v1
OK
localhost:7379> SET k w1
OK
localhost:7379> GET.WATCH k
entered the watch mode for GET.WATCH k 
OK [fingerprint=6645377565653310095] "w3"
^Cexiting the watch mode. back to command mode
localhost:7379>  GET a
OK "v1"
localhost:7379> GET k
OK "w3"
localhost:7379> GET.WATCH k
entered the watch mode for GET.WATCH k
OK [fingerprint=6645377565653310095] "w3" 
^Cexiting the watch mode. back to command mode
localhost:7379> GET a
OK [fingerprint=6645377565653310095] "e1"
localhost:7379> GET k
OK [fingerprint=6645377565653310095] "e2"

Related PR's:

Fixes: #1696


  • Simplified ClientID assignment logic to ensure it is set directly without error checks after the initial loop.
  • Updated HANDSHAKE command handling to directly assign ClientID and Mode without redundant error checks.
  • Modified watch command handling to ensure proper notification of watchers without unnecessary conditions.
  • Enhanced cleanup logic in WatchManager to delete command entries when fingerprints are removed.

These changes improve code clarity and maintainability while ensuring correct behavior during command processing.

…ments

- Simplified ClientID assignment logic to ensure it is set directly without error checks after the initial loop.
- Updated HANDSHAKE command handling to directly assign ClientID and Mode without redundant error checks.
- Modified watch command handling to ensure proper notification of watchers without unnecessary conditions.
- Enhanced cleanup logic in WatchManager to delete command entries when fingerprints are removed.

These changes improve code clarity and maintainability while ensuring correct behavior during command processing.
- Added error handling for the thread.Stop() method to log failures when stopping the IO thread.
- Enhanced logging to include client ID and mode for better traceability in case of errors.
…d update NotifyWatchers signature to include IOThread instance.
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.

Running Watch command after closing first one doesn't work
1 participant