Skip to content

[opt](point-query) reduce short circuit lookup cache usage #67228

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

Open
HonestManXin wants to merge 1 commit into
base: master
Choose a base branch
from
+29 −1
Open

[opt](point-query) reduce short circuit lookup cache usage #67228

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/qe/ShortCircuitQueryContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,30 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.protobuf.ByteString;
import org.apache.thrift.TException;
import org.apache.thrift.TSerializer;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;

public class ShortCircuitQueryContext {
// Number of buckets used to spread a hot query over multiple Backend
// LookupConnectionCache shards, avoiding single-shard lock contention.
private static final int CACHE_ID_BUCKET_NUM = 128;

// Round-robin bucket allocator, giving an even distribution across buckets
// regardless of connection id skew.
private static final AtomicLong CACHE_ID_BUCKET_COUNTER = new AtomicLong(0);

// Cached for better CPU performance, since serialize DescriptorTable and
// outputExprs are heavy work
public final Planner planner;
Expand Down Expand Up @@ -110,14 +122,30 @@ public ShortCircuitQueryContext(Planner planner, Queriable analzyedQuery) throws
TExprList exprList = new TExprList(exprs);
serializedOutputExpr = ByteString.copyFrom(
new TSerializer().serialize(exprList));
this.cacheID = UUID.randomUUID();
this.cacheID = genCacheID(serializedDescTable, serializedOutputExpr, serializedQueryOptions);

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Do not share this mutable BE context across connections. The first and 129th identical prepared contexts on one FE (or bucket 0 on two FEs) now use the same UUID, so concurrent BE requests receive the same Reusable. Only its block pool is locked: each request writes runtime_state()->set_timezone(...) and both execute the same original VExprContextSPtrs, whose execution mutates context/function state. For example, A can set UTC, B overwrite Asia/Tokyo, and A's supported from_unixtime point query formats with B's timezone; concurrent string/timezone access is also a C++ data race. Make the cached value immutable and clone/lease request-local runtime and expression state (or otherwise serialize the whole use) before collapsing IDs; adding timezone to the hash alone does not make same-timezone executions thread-safe.

this.scanNode = olapScanNode;
this.tbl = this.scanNode.getOlapTable();
this.tableName = this.scanNode.getTableNameInPlan();
this.schemaVersion = this.tbl.getBaseSchemaVersion();
this.analzyedQuery = analzyedQuery;
}

// Build a 128-bit cache identifier from serialized query structures and a
// round-robin bucket. Identical hot queries are intentionally spread across
// multiple Backend LookupConnectionCache shards to reduce lock contention and
// high sys CPU, while still bounding the number of cache entries.
private static UUID genCacheID(ByteString serializedDescTable, ByteString serializedOutputExpr,
ByteString serializedQueryOptions) {
int bucket = (int) Math.floorMod(CACHE_ID_BUCKET_COUNTER.getAndIncrement(), CACHE_ID_BUCKET_NUM);

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Rotate buckets per query identity, not from one JVM-global ordinal. With a fixed set of N prepared contexts created in the same order on every connection, query j gets (N * connection + j) mod 128 and reaches only 128 / gcd(N, 128) buckets. For 128 statements, every instance of each hot query uses one UUID and one BE shard, recreating the single-shard contention this code claims to avoid even though the global bucket histogram is even. Scope the sequence to the unhashed query identity (or use another proven per-query spread) and test repeated multi-statement connection initialization.

Hasher hasher = Hashing.murmur3_128().newHasher();

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Generate this ID only when a request can actually send it. StmtExecutor creates this context for every text-protocol short-circuit query, but PointQueryExecutor.buildLookupRequest sends cacheID only for COM_STMT_EXECUTE, so every COM_QUERY copies and scans the full plan for no BE benefit. There is a second dead path for nondeterministic prepared statements: execution builds the context it sends, then ExecuteCommand stores another context after execution even though its hasNondeterministic() guard prevents that retained context from ever taking the direct path. Both replace fixed-size random-UUID work with full-payload copies and hashing. Make ID generation lazy/request-driven and test both unsent paths.

hasher.putBytes(serializedDescTable.toByteArray());

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Hash the existing ByteString views instead of allocating three full copies. Each toByteArray() duplicates a payload that this context already owns, so 10,000 wide-table contexts create three large transient arrays apiece solely for Murmur input and add avoidable GC pressure to the workload being optimized. Guava's hasher can consume ByteBuffer; use serialized...asReadOnlyByteBuffer() (or another zero-copy view) for contexts that actually need an ID.

hasher.putBytes(serializedOutputExpr.toByteArray());

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Preserve the nondeterministic-plan no-reuse boundary in this ID. ExecuteCommand deliberately avoids reusing its retained short-circuit context when hasNondeterministic() is true, but a fresh execution still hashes the same BE TExpr: FE's volatile identity is not serialized. Two FEs starting at bucket 0 can therefore send the same UUID for random(7). The first miss opens and seeds Random's cached THREAD_LOCAL mt19937_64; the later sequential hit reuses that already-open function context and advances the first statement's generator instead of reseeding to 7. This needs no race, and serializing the shared context would not fix it. Keep nondeterministic contexts uniquely keyed or clone/open request-local expression/function state, with a sequential warm-cache seeded-random test across matching buckets.

hasher.putBytes(serializedQueryOptions.toByteArray());

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Include the schema generation in this cache identity. isReusable deliberately invalidates an FE context when baseSchemaVersion changes, but the new UUID omits that value. A row_store_columns schema change can keep the serialized descriptor/output/options byte-identical while BE's Reusable::init derives different include_col_uids/missing_col_uids from the new TabletSchema. If the 128 old hot IDs are resident, refreshed contexts immediately hit those stale objects; a column removed from row storage is then not fetched from column storage and can be returned as a default/wrong value. Hash a complete schema/version token and/or validate cached state against the request tablet schema, with a warm-cache row-store schema-change test.

hasher.putInt(bucket);

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Keep wide-block work out of the newly shared per-key mutex. Once independent connections converge on one of these 128 IDs, every warm hit uses the same Reusable::_block_mutex. Each pool has only 32 blocks; when it empties, get_block() allocates a wide block while holding the lock, and return_block() clears every column and may destroy an excess block while still locked. In the stated 10,000-request burst, about 78 borrowers share each ID, so at least 46 allocations serialize per pool and the excess blocks are destroyed before the next burst repeats that work. Move only the vector pop/push under the lock, doing allocation, clearing, and over-capacity destruction after unlocking (or use request-local/striped pools), with a >32-borrower warm-cache contention test.

ByteBuffer buffer = ByteBuffer.wrap(hasher.hash().asBytes());
return new UUID(buffer.getLong(), buffer.getLong());

@github-actions github-actions Bot Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Coalesce cold initialization for the new shared keys. BE currently does get(uuid), deserializes and runs Reusable::init(..., 32), then add(uuid) with no per-key single-flight; duplicate insertion is last-writer-wins. During a 10,000-connection cold start, roughly 78 requests per new key can all miss and allocate the wide descriptors, expression state, and 32 blocks before the cache converges, preserving an O(connection-count) transient memory/CPU spike. Add lookup-or-create/single-flight coordination with a barrier-based same-key miss test.

}

@VisibleForTesting
ShortCircuitQueryContext(OlapTable tbl, String tableName, int schemaVersion,
long fileCacheQueryLimitBytes) {
Expand Down
Loading