-
Notifications
You must be signed in to change notification settings - Fork 0
MOD-10359 Implement shard window ratio optimization #7
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
base: 6383_base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,9 @@ | |
| #include "util/timeout.h" | ||
| #include "resp3.h" | ||
| #include "coord/config.h" | ||
| #include "config.h" | ||
| #include "dist_profile.h" | ||
| #include "shard_window_ratio.h" | ||
| #include "util/misc.h" | ||
| #include "aggregate/aggregate_debug.h" | ||
| #include "info/info_redis/threads/current_thread.h" | ||
|
|
@@ -529,7 +531,7 @@ static RPNet *RPNet_New(const MRCommand *cmd) { | |
| } | ||
|
|
||
| static void buildMRCommand(RedisModuleString **argv, int argc, int profileArgs, | ||
| AREQDIST_UpstreamInfo *us, MRCommand *xcmd, IndexSpec *sp) { | ||
| AREQDIST_UpstreamInfo *us, MRCommand *xcmd, IndexSpec *sp, specialCaseCtx *knnCtx) { | ||
| // We need to prepend the array with the command, index, and query that | ||
| // we want to use. | ||
| const char **tmparr = array_new(const char *, us->nserialized); | ||
|
|
@@ -608,6 +610,22 @@ static void buildMRCommand(RedisModuleString **argv, int argc, int profileArgs, | |
| } | ||
| } | ||
|
|
||
| // Handle KNN with shard ratio optimization for both multi-shard and standalone | ||
| if (knnCtx) { | ||
| KNNVectorQuery *knn_query = &knnCtx->knn.queryNode->vn.vq->knn; | ||
| double ratio = knn_query->shardWindowRatio; | ||
|
|
||
| if (ratio < MAX_SHARD_WINDOW_RATIO) { | ||
| // Apply optimization only if ratio is valid and < 1.0 (ratio = 1.0 means no optimization) | ||
| // Calculate effective K based on deployment mode | ||
| size_t numShards = GetNumShards_UnSafe(); | ||
| size_t effectiveK = calculateEffectiveK(knn_query->k, ratio, numShards); | ||
|
|
||
| // Modify the command to replace KNN k (shards will ignore $SHARD_K_RATIO) | ||
| modifyKNNCommand(xcmd, 2 + profileArgs, effectiveK, knnCtx->knn.queryNode->vn.vq); | ||
| } | ||
| } | ||
|
Comment on lines
+613
to
+627
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [BestPractice] This block of logic for handling the shard window ratio is very similar to the logic in The implementation in |
||
|
|
||
| // check for timeout argument and append it to the command. | ||
| // If TIMEOUT exists, it was already validated at AREQ_Compile. | ||
| int timeout_index = RMUtil_ArgIndex("TIMEOUT", argv + 3 + profileArgs, argc - 4 - profileArgs); | ||
|
|
@@ -713,11 +731,14 @@ static int prepareForExecution(AREQ *r, RedisModuleCtx *ctx, RedisModuleString * | |
| r->profile = printAggProfile; | ||
|
|
||
| unsigned int dialect = r->reqConfig.dialectVersion; | ||
| specialCaseCtx *knnCtx = NULL; | ||
|
|
||
| if(dialect >= 2) { | ||
| // Check if we have KNN in the query string, and if so, parse the query string to see if it is | ||
| // a KNN section in the query. IN that case, we treat this as a SORTBY+LIMIT step. | ||
| if(strcasestr(r->query, "KNN")) { | ||
| specialCaseCtx *knnCtx = prepareOptionalTopKCase(r->query, argv, argc, dialect, status); | ||
| // For distributed aggregation, command type detection is automatic | ||
| knnCtx = prepareOptionalTopKCase(r->query, argv, argc, dialect, status); | ||
| *knnCtx_ptr = knnCtx; | ||
| if (QueryError_HasError(status)) { | ||
| return REDISMODULE_ERR; | ||
|
|
@@ -739,7 +760,7 @@ static int prepareForExecution(AREQ *r, RedisModuleCtx *ctx, RedisModuleString * | |
|
|
||
| // Construct the command string | ||
| MRCommand xcmd; | ||
| buildMRCommand(argv , argc, profileArgs, &us, &xcmd, sp); | ||
| buildMRCommand(argv , argc, profileArgs, &us, &xcmd, sp, knnCtx); | ||
| xcmd.protocol = is_resp3(ctx) ? 3 : 2; | ||
| xcmd.forCursor = r->reqflags & QEXEC_F_IS_CURSOR; | ||
| xcmd.forProfiling = IsProfile(r); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| * Copyright (c) 2006-Present, Redis Ltd. | ||
| * All rights reserved. | ||
| * | ||
| * Licensed under your choice of the Redis Source Available License 2.0 | ||
| * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the | ||
| * GNU Affero General Public License v3 (AGPLv3). | ||
| */ | ||
| #pragma once | ||
|
|
||
| #include "util/heap.h" | ||
| #include "query_node.h" | ||
|
|
||
| typedef enum { | ||
| SPECIAL_CASE_NONE, | ||
| SPECIAL_CASE_KNN, | ||
| SPECIAL_CASE_SORTBY | ||
| } searchRequestSpecialCase; | ||
|
|
||
| typedef struct { | ||
| size_t k; // K value TODO: consider remove from here, its in querynode | ||
| const char* fieldName; // Field name | ||
| bool shouldSort; // Should run presort before the coordinator sort | ||
| size_t offset; // Reply offset | ||
| heap_t *pq; // Priority queue | ||
| QueryNode* queryNode; // Query node | ||
| } knnContext; | ||
|
|
||
| typedef struct { | ||
| const char* sortKey; // SortKey name; | ||
| bool asc; // Sort order ASC/DESC | ||
| size_t offset; // SortKey reply offset | ||
| } sortbyContext; | ||
|
|
||
| typedef struct { | ||
| union { | ||
| knnContext knn; | ||
| sortbyContext sortby; | ||
| }; | ||
| searchRequestSpecialCase specialCaseType; | ||
| } specialCaseCtx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Documentation]
This comment is slightly confusing. While technically correct that it's an exclusive minimum, it could be clearer. Consider rephrasing to directly state the requirement for

ratio.