8000 Convert pubsub dicts to hashtables by zuiderkwast · Pull Request #2007 · valkey-io/valkey · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Convert pubsub dicts to hashtables #2007

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 27, 2025
Merged

Convert pubsub dicts to hashtables #2007

merged 2 commits into from
Apr 27, 2025

Conversation

zuiderkwast
Copy link
Contributor
@zuiderkwast zuiderkwast commented Apr 25, 2025 •
edited
Loading

Two dicts are converted to hashtables:

  1. On each client, the set of channels/patterns/shard-channels the client is subscribed to
  2. On each channel or pattern, the set of clients subscribed to it.

When this is done, we can remove a lot of complexity in dict.c including the pointer tricks for different kinds of dictEntry, because that will become dead code. We can delete the dead code in a separate PR.

@zuiderkwast zuiderkwast added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Apr 25, 2025
@zuiderkwast zuiderkwast force-pushed the pubsub-dicts-to-hashtables branch from ca5d963 to 467ae6a Compare April 25, 2025 14:45
SoftlyRaining and others added 2 commits April 25, 2025 16:57
@SoftlyRaining @zuiderkwast
Convert client pubsub channels dicts to hashtables
533bac1 
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast
Convert pubsub channel's set of clients dicts to hashtables
d73d53f 
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@zuiderkwast zuiderkwast force-pushed the pubsub-dicts-to-hashtables branch from 467ae6a to d73d53f Compare April 25, 2025 14:57
@zuiderkwast zuiderkwast requested review from ranshid and hpatro and removed request for ranshid April 25, 2025 15:12
@codecov Codecov
Copy link
codecov bot commented Apr 25, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 87.69231% with 16 lines in your changes missing coverage. Please review.

Project coverage is 71.01%. Comparing base (30e9109) to head (d73d53f).
Report is 4 commits behind head on unstable.

Files with missing lines Patch % Lines
src/defrag.c 0.00% 16 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##           unstable    #2007   +/-   ##
=========================================
  Coverage     71.01%   71.01%           
=========================================
  Files           123      123           
  Lines         66059    66048   -11     
=========================================
- Hits          46911    46904    -7     
+ Misses        19148    19144    -4
Files with missing lines Coverage Δ
src/acl.c 90.72% <100.00%> (+0.02%) ⬆️
src/networking.c 87.29% <ø> (ø)
src/pubsub.c 96.79% <100.00%> (-0.03%) ⬇️
src/server.c 87.94% <100.00%> (+<0.01%) ⬆️
src/server.h 100.00% <ø> (ø)
src/defrag.c 81.53% <0.00%> (+0.55%) ⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ranshid
ranshid approved these changes Apr 27, 2025
View reviewed changes
Copy link
Member
@ranshid ranshid left a comment

Choose a reason for hiding this comment

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

LGTM

@zuiderkwast zuiderkwast merged commit 249495a into valkey-io:unstable Apr 27, 2025
60 checks passed
@zuiderkwast zuiderkwast deleted the pubsub-dicts-to-hashtables branch April 27, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hpatro hpatro hpatro approved these changes

@ranshid ranshid ranshid approved these changes

Assignees
No one assigned
Labels
run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)
Projects
None yet
Milestone
No milestone
3A4C
Development

Successfully merging this pull request may close these issues.
4 participants
@zuiderkwast @hpatro @ranshid @SoftlyRaining
0