[xiangfu0]

Summary

The gRPC broker transport dialed with only transport credentials and never set grpc.MaxCallRecvMsgSize, so it inherited gRPC's default 4MB maxReceiveMessageSize. Pinot streams results as DataBlock messages whose size scales with blockRowSize (default 10000 rows, set in buildGrpcMetadata) and column width, so a single block can exceed 4MB and fail stream.Recv() with ResourceExhausted: received message larger than max — on queries the HTTP transport handles fine. The Java gRPC client sets this limit explicitly.

Changes

• Add optional MaxRecvMsgSizeBytes field to GrpcConfig.
• Configure grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(...)) in buildGrpcDialOptions, defaulting to 128MB when unset.
• Extend TestBuildGrpcDialOptions to cover the default and explicit-value paths.

Backward compatible: zero-value GrpcConfig now accepts larger result blocks automatically; public API is unchanged (new optional field only).

Reproduction

Run a SELECT returning ~10000 rows with large STRING/JSON/ARRAY columns through the gRPC transport. Before this change the client errors at stream.Recv() with a ResourceExhausted message-too-large error; after it, the stream is accepted up to the configured (default 128MB) ceiling.

Validation

• make lint — 0 issues
• make test — all pass, race + atomic coverage; pinot package coverage 99.7% (unchanged)
• go build ./..., go vet ./... — clean

Integration tests (live Pinot quickstart) were not re-run: this change only affects gRPC dial configuration and existing current-head CI covers the integration path.

Fixes #104

[intentlab-ai]

Code Review

✅ Passed

_{Reviewed 4d6e006}

[intentlab-ai]

PR Readiness Review

✅ Passed

_{Reviewed 4d6e006}

[intentlab-ai]

Code Health Review

✅ Passed

Existing debt (14)

• Inherited · Cognitive Complexity — pinot/grpcBrokerClientTransport.go::readArrowValue
  cognitive complexity is 73; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cognitive Complexity — pinot/grpcBrokerClientTransport.go::decompressGrpcPayload
  cognitive complexity is 53; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cognitive Complexity — pinot/grpcBrokerClientTransport.go::execute
  cognitive complexity is 45; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cognitive Complexity — gormpinot/rows.go::convertValue
  cognitive complexity is 44; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cognitive Complexity — gormpinot/dialector.go::QuoteTo
  cognitive complexity is 32; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cyclomatic — pinot/grpcBrokerClientTransport.go::readArrowValue
  cyclomatic is 31; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cyclomatic — gormpinot/rows.go::convertValue
  cyclomatic is 22; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cyclomatic — pinot/grpcBrokerClientTransport.go::execute
  cyclomatic is 22; limit is 20.
  Reduce this metric below the limit.
• Inherited · Cyclomatic — pinot/grpcBrokerClientTransport.go::decompressGrpcPayload
  cyclomatic is 21; limit is 20.
  Reduce this metric below the limit.
• Inherited · Nesting Max — gormpinot/dialector.go::QuoteTo
  nesting max is 5; limit is 4.
  Reduce this metric below the limit.
• Inherited · Nesting Max — pinot/dynamicBrokerSelector.go::setupWatcher
  nesting max is 5; limit is 4.
  Reduce this metric below the limit.
• Inherited · Nesting Max — pinot/grpcBrokerClientTransport.go::decodeArrowRows
  nesting max is 5; limit is 4.
  Reduce this metric below the limit.
• Inherited · Clone family — gormpinot/rows.go:151-158
  Near-duplicate functions (100% normalized-structure similarity). Members: gormpinot/rows.go:151-158 (isFloatType); gormpinot/rows.go:142-149 (isIntType).
  To resolve: extract the duplicated logic into a single function and call it from every member, or fold near-duplicates into a parameterised helper. Re-run the analyzer to confirm the family is gone.
• Inherited · Clone family — pinot/grpcBrokerClientTransport.go:480-495
  Near-duplicate functions (88% normalized-structure similarity). Members: pinot/grpcBrokerClientTransport.go:480-495 (decodeArrowBoolList); pinot/grpcBrokerClientTransport.go:548-563 (decodeArrowDoubleList); pinot/grpcBrokerClientTransport.go:531-546 (decodeArrowFloatList); pinot/grpcBrokerClientTransport.go:497-512 (decodeArrowIntList); +2 more member(s).
  To resolve: extract the duplicated logic into a single function and call it from every member, or fold near-duplicates into a parameterised helper. Re-run the analyzer to confirm the family is gone.

_{Reviewed 4d6e006}

[xiangfu0]

Independent code review (maintenance routine)

Reviewed head 1af7724. This is an independent review for the record — not a GitHub approval.

Verdict: no critical or major issues. The fix is correct, minimal, and purely additive.

• Correctness ✓ — grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(...)) is the right API and is applied to the server-streaming stream.Recv() path that consumes Pinot result blocks, lifting gRPC's 4MB default. MaxSendMsgSize is correctly left untouched (the client only sends a small SQL+metadata request). <= 0 correctly clamps to the 128MB default (also avoids gRPC's "negative = unlimited" footgun).
• Backward compatible ✓ — one new exported field on GrpcConfig; zero-value triggers the default. No existing call sites or behavior change (beyond the intended raised ceiling).

Minor / nits recorded for optional follow-up (non-blocking):

1. assert.Len(options, 2) verifies the option count, not which option was added or its value; a behavioral test (mock server returning a >4MB block; a tiny MaxRecvMsgSizeBytes that triggers ResourceExhausted) would be stronger. The existing startGrpcTestServer harness supports this.
2. Raising the per-message cap to 128MB increases the memory feeding the pre-existing un-ceilinged decompressGrpcPayload path (io.ReadAll of the decompressed stream). Acceptable given the broker is trusted; pre-existing, not introduced here.
3. 32-bit int ceiling would cap user-supplied values >2GB — not reachable by the default; non-issue in practice.

All CI is green on this head (unit, integration, linter, CodeQL, Snyk, coveralls). Normal squash auto-merge has been enabled; the PR will merge once the GitHub-required approving review is provided.

[xiangfu0]

Rebased onto current master (5eb5385) to resolve the out-of-date base. The only delta master gained beyond this PR's original base was a CI-only actions/setup-java bump, so the fix is unchanged.

Also strengthened the test for the new behavior per independent review: extracted resolveMaxRecvMsgSize and added assertions that it defaults to 128MB when the config is unset/<= 0 and honors an explicit positive value — so the gRPC 4MB default can never silently apply undetected. Public API and runtime behavior are unchanged.

Local validation on the rebased head (4d6e006):

• make lint — 0 issues
• make test — all pass (race + atomic coverage); pinot 99.7%, pinot/proto 100.0%
• go build ./..., go vet ./... — clean

CI is re-running on the new head.