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

Python: adds DBSIZE command #1040

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Python: adds DBSIZE command #1040

merged 3 commits into from
Mar 20, 2024

Conversation

shohamazon
Copy link
Member

Issue #, if available:

Description of changes:
"request_policy:all_shards"
"response_policy:agg_sum"

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@shohamazon shohamazon requested a review from a team as a code owner February 26, 2024 13:19
in which case the client will route the command to the nodes defined by `route`.

Returns:
int: The number of keys in the currently selected database.
Copy link
Contributor

Choose a reason for hiding this comment

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

"currently selected database" is for a standalone description where there's more than a single DB in a server.
Suggested:
int: The number of keys in the database. In the case of routing the query to multiple nodes, returns the aggregated number of keys across the different nodes.


Examples:
>>> await client.dbsize()
10 # Indicates there are 10 keys in the current database.
Copy link
Contributor

Choose a reason for hiding this comment

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

not accurate. When routing isn't provided we're sending this command to multiple nodes (all primaries), the comment should say that

See https://redis.io/commands/dbsize for more details.

Commands response:
int: The number of keys in the currently selected database.
Copy link
Contributor

Choose a reason for hiding this comment

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

int: The number of keys in the database.

async def test_dbsize(self, redis_client: TRedisClient):
assert await redis_client.custom_command(["FLUSHALL"]) == OK

key_value_pairs = [(get_random_string(10), "foo") for _ in range(10)]
Copy link
Contributor

Choose a reason for hiding this comment

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

move it under the assertion in 1268

cluster_nodes = get_first_result(cluster_nodes)
replica_count = cluster_nodes.count("slave")
master_count = cluster_nodes.count("master")
assert await redis_client.dbsize(AllNodes()) == 10 * (
Copy link
Contributor

Choose a reason for hiding this comment

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

can be flakey if some of the replicas haven't finished syncing yet.. instead you can test with SlotKeyRoute:

  1. flushall
  2. setting specific key
  3. routing dbsize to this specific key and making sure it returns 1

Copy link
Contributor

Choose a reason for hiding this comment

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

you can do the same with a transaction containing flush, set, dbsize. might be a bit shorter.

Copy link
Contributor

@barshaul barshaul left a comment

Choose a reason for hiding this comment

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

fix & merge

@shohamazon shohamazon merged commit 1ada44a into aws:main Mar 20, 2024
42 of 45 checks passed
@shohamazon shohamazon deleted the python/dbsize branch March 20, 2024 13:08
Sa1Gur pushed a commit to Sa1Gur/glide-for-redis that referenced this pull request Mar 25, 2024
cyip10 pushed a commit to Bit-Quill/glide-for-redis that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants