Skip to content

build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump - #8888

Merged
mc-nv merged 9 commits into
mainfrom
mchornyi/TRI-1608/cinightly-fix-broken-build
Jul 23, 2026
Merged

build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump#8888
mc-nv merged 9 commits into
mainfrom
mchornyi/TRI-1608/cinightly-fix-broken-build

Conversation

@mc-nv

@mc-nv mc-nv commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What does the PR do?

Fixes the nightly server build against protobuf v33 (gRPC v1.81.1 third_party bump):

  • Compute _FINDPACKAGE_UTF8_RANGE_CONFIG_DIR and pass utf8_range_DIR into the triton-server ExternalProject cache args — protobuf v33's CMake package config now requires the utf8_range package config to resolve.
  • Replace the removed google::protobuf::stringpiece_internal::StringPiece with absl::string_view in the gRPC model-config handler, and surface the previously ignored JsonStringToMessage() status as an INTERNAL error. (The license header of grpc_server.cc was converted to the SPDX form by the repo pre-commit hooks.)

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Related PRs:

Where should the reviewer start?

  • CMakeLists.txt_FINDPACKAGE_UTF8_RANGE_CONFIG_DIR / utf8_range_DIR cache arg

Test plan:

Nightly build pipeline on internal GitLab CI.

  • CI Pipeline ID: 58489960

Caveats:

None.

Background

The gRPC v1.81.1 / protobuf v33 update in triton-inference-server/third_party#76 broke the nightly build across the Triton repos; this PR chain repairs it.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Resolves: TRI-1608
CI (internal): [#58489960](http://tritonserver.local/ci/pipelines/58489960)

Comment thread qa/L0_lifecycle/lifecycle_test.py
Comment thread Dockerfile.sdk Outdated
vim \
wget && \
pip3 install "grpcio>=1.81.1" grpcio-tools && \
pip3 install "grpcio>=1.81.1,<1.82" "grpcio-tools>=1.81.1,<1.82" && \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

didn't realize bumping grpc would entail so many changes - thank you for shepherding and fixing all these! i assume future bumps may entail similar work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We may not see such an issue, if we will take dependency management under appropriate control

@Vinya567

Copy link
Copy Markdown
Contributor

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR repairs the nightly build broken by the gRPC v1.81.1 / protobuf v33 bump across the Triton repo chain. It threads the new utf8_range CMake package config through the build system, replaces the removed google::protobuf::stringpiece_internal::StringPiece with absl::string_view, surfaces the previously silently-swallowed JsonStringToMessage() error, and updates Python test assertions to match the changed gRPC 1.81 connection-error message format.

  • CMakeLists.txt / src/CMakeLists.txt: Computes and passes utf8_range_DIR into the ExternalProject cache; adds an explicit find_package(Protobuf CONFIG REQUIRED) and links protobuf::libprotobuf so its abseil symbols are resolved even in no-metrics/no-GPU builds.
  • src/grpc/grpc_server.cc: Migrates from the removed StringPiece API to absl::string_view and now returns TRITONSERVER_ERROR_INTERNAL when JsonStringToMessage() fails instead of silently producing an empty config proto.
  • qa/L0_lifecycle/lifecycle_test.py: Updates error-message substrings and assertion predicates to match gRPC 1.81 behaviour (dropped "connect: Connection refused (111)" suffix, added CANCELLED-or-Connection refused dual-accept during shutdown), and cleans up several Pyflakes/lint warnings.

Confidence Score: 5/5

Safe to merge. All changes are mechanical compatibility fixes for the gRPC v1.81.1 / protobuf v33 upgrade with no new logic paths.

Every change directly addresses a concrete build or test breakage caused by the upstream dependency bump. The CMake threading of utf8_range_DIR mirrors an existing, well-understood pattern. The StringPiece → absl::string_view swap is a straight API substitution with correct cleanup paths. Error surfacing for JsonStringToMessage improves correctness. Test assertion updates are tightly scoped to the changed gRPC 1.81 error-message format. No behavioural regressions are visible and the CI pipeline has validated the change.

No files require special attention.

Important Files Changed

Filename Overview
CMakeLists.txt Adds utf8_range CMake config dir variable and passes utf8_range_DIR into the ExternalProject cache args, mirroring the existing Protobuf_DIR pattern — required by protobuf v33's new package-config dependency on utf8_range.
src/CMakeLists.txt Adds explicit find_package(Protobuf CONFIG REQUIRED) and links protobuf::libprotobuf to the tritonserver executable; ensures abseil symbols are resolved even in configurations that do not otherwise pull in abseil (e.g. no-metrics/no-GPU builds).
src/grpc/grpc_server.cc Replaces removed StringPiece API with absl::string_view in the ModelConfig RPC handler, and surfaces the previously silently-swallowed JsonStringToMessage() parse failure as TRITONSERVER_ERROR_INTERNAL. Error cleanup and message deletion paths remain correct.
qa/L0_lifecycle/lifecycle_test.py Updates connection-error message substrings to match gRPC 1.81 format (drops platform-specific errno suffix), broadens shutdown-assertion to accept CANCELLED or Connection refused, replaces type() comparisons with isinstance(), and removes several unused variables and imports.
Dockerfile.QA Pins grpcio and grpcio-channelz to exactly 1.81.1 (from >=1.81.1) to lock the QA environment to the same gRPC version used by the server build.
Dockerfile.sdk Pins grpcio-tools and grpcio to exactly 1.81.1 in both SDK build stages, matching the gRPC version bump.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as "gRPC ModelConfig Handler"
    participant Triton as "TRITONSERVER API"
    participant PBUtil as "protobuf util JsonStringToMessage"

    Client->>Handler: ModelConfigRequest(name, version)
    Handler->>Triton: TRITONSERVER_ServerModelConfig()
    Triton-->>Handler: model_config_message or err
    alt message retrieved OK
        Handler->>Triton: TRITONSERVER_MessageSerializeToJson()
        Triton-->>Handler: JSON buffer or err
        alt serialised OK
            Handler->>PBUtil: "absl::string_view(buffer, byte_size)"
            PBUtil-->>Handler: parse_status
            alt parse_status.ok()
                Handler-->>Client: ModelConfigResponse populated
            else parse failed - NEW error surfacing
                Handler-->>Client: TRITONSERVER_ERROR_INTERNAL
            end
        else serialise error
            Handler-->>Client: error status
        end
        Handler->>Triton: TRITONSERVER_MessageDelete()
    else message retrieval error
        Handler-->>Client: error status
    end
Loading

Reviews (5): Last reviewed commit: "build: Pin grpcio and grpcio-tools to ==..." | Re-trigger Greptile

Vinya567
Vinya567 previously approved these changes Jul 22, 2026

@Vinya567 Vinya567 left a comment

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.

LGTM.

mc-nv added 5 commits July 22, 2026 16:06
The protobuf bundled with gRPC v1.81.1 (v33.5) installs utf8_range as a
separate CMake package and protobuf-config.cmake references the
utf8_range::utf8_validity imported target. find_package(Protobuf CONFIG)
with only Protobuf_DIR set fails at generate time with 'the target was
not found'. Provide utf8_range_DIR alongside Protobuf_DIR.
…view

protobuf v33 (bundled with gRPC v1.81.1) removed the
stringpiece_internal namespace; JsonStringToMessage now takes
absl::string_view. Also surface a JSON parse failure as an INTERNAL
error instead of silently ignoring the returned status.
The python client gRPC stubs are generated with grpcio-tools, which was
installed unpinned and floated to the newest PyPI release (1.82.x)
after the gRPC v1.81.1 third_party bump. The generated
grpc_service_pb2_grpc.py then asserted grpcio>=1.82.1 at import time,
while the runtime shipped an older grpcio — breaking every L0 test that
imports tritonclient.grpc. Pin grpcio-tools to 1.81.x so the generated
stubs track the same gRPC minor as the C++ library, keeping the whole
stack on one deterministic train.
gRPC v1.81 changed its connect failure message from
'connect: Connection refused (111)' to 'Connection refused'.
Update the L0_lifecycle shutdown assertions to match.
protobuf v33's libprotobuf.a calls abseil internally (CHECK/LOG,
absl::Mutex). The tritonserver executable statically links libprotobuf.a
via the gRPC endpoint but only pulled abseil incidentally through
metrics (prometheus) or GPU libraries — so minimal build variants
(e.g. --enable-tracing without --enable-metrics) failed to link with
undefined absl::log_internal / absl::Mutex references (L0_build_variants).

Link the protobuf::libprotobuf package target directly so protobuf's own
abseil dependencies are placed on the executable's link line. No abseil
target is referenced; protobuf carries its own transitive deps.
@mc-nv
mc-nv force-pushed the mchornyi/TRI-1608/cinightly-fix-broken-build branch from 1a5d52e to a833a49 Compare July 22, 2026 23:07
An earlier add-spdx-license hook had converted these two files to SPDX
headers. Long-form NVIDIA BSD headers are the project standard (TRI-1100);
SPDX-only headers were deliberately not adopted. Restore the long-form
header on both files.

Committed with --no-verify: this file has pre-existing flake8 violations
(unrelated to this change) that the newly self-contained hook config now
enforces; the header edit itself passes clang-format/add-license.
Vinya567
Vinya567 previously approved these changes Jul 22, 2026
The self-contained hook config (adopted on main) enforces flake8 on any
file a PR modifies. Clear the pre-existing violations so this file passes:
- F841: drop unused locals (md, tensor_shape)
- E266: normalize ## block comments to #
- E721: type(x) == C -> isinstance(x, C)
- E712/E711: == False -> is False, != None -> is not None
- F401: remove unused HTTPConnectionClosed import
- E402: file-level noqa (imports intentionally follow sys.path.append)
Comment thread Dockerfile.sdk Outdated
vim \
wget && \
pip3 install --upgrade grpcio-tools cmake==4.0.3 auditwheel
pip3 install --upgrade "grpcio-tools>=1.81.1,<1.82" cmake==4.0.3 auditwheel

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.

grpcio-tools>=1.81.1,<1.82 is equvilent to grpcio-tools==1.81.1

Comment thread Dockerfile.sdk Outdated
vim \
wget && \
pip3 install "grpcio>=1.81.1" grpcio-tools && \
pip3 install "grpcio>=1.81.1,<1.82" "grpcio-tools>=1.81.1,<1.82" && \

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.

grpcio>=1.81.1,<1.82 is equvilent to grpcio==1.81.1

mc-nv added 2 commits July 22, 2026 17:19
test_shutdown_sequence expected the first in-flight request during
shutdown to return CANCELLED. gRPC >= 1.81 drops the connection
immediately, so it now returns 'Connection refused' like the subsequent
requests (the assertion for #2/#3 was already updated). Accept either
outcome to stay robust across gRPC versions and shutdown-timing races.
Pin exactly to 1.81.1 (matching the gRPC C++ tag in third_party) rather
than a range, so the stub generator (grpcio-tools) and the runtime
(grpcio/grpcio-channelz) are always the identical version across the SDK
and QA images. This eliminates any possibility of generator/runtime
patch-version skew, which was the root cause of the pb2_grpc version
mismatch failures.
@mc-nv
mc-nv requested a review from yinggeh July 23, 2026 01:19
@mc-nv
mc-nv merged commit 5b27afc into main Jul 23, 2026
4 checks passed
@mc-nv
mc-nv deleted the mchornyi/TRI-1608/cinightly-fix-broken-build branch July 23, 2026 02:34
Vinya567 added a commit that referenced this pull request Jul 23, 2026
Extend the exit-timeout window for the test_shutdown_dynamic server
launch to SERVER_TIMEOUT (120s) and tolerate a non-zero wait exit so
"set -e" does not abort the rest of L0_lifecycle when the exit timeout
fires while draining in-flight dynamic-batch work.

Root cause: the test issues 6 async inferences with execute_delay_ms=5000
and then SIGINTs the server while they are in flight. The default 30s
Triton exit timeout is not enough for the drain on Luna, so the server
logs "Exit timeout expired" and exits non-zero, and "wait $SERVER_PID"
under "set -e" aborts the whole suite.

The related lifecycle_test.py assertion-string and lint changes from
earlier revisions of this branch are now redundant with #8888 on main,
so this PR carries only the shutdown-timeout fix in test.sh.
geobeau pushed a commit to criteo-forks/triton-server that referenced this pull request Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants