Unify the deframer of the client and server sides for the Triple protocol #16041

EarthChen · 2026-01-22T14:08:48Z

What is the purpose of the change?

Checklist

Make sure there is a GitHub_issue field for the change.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

EarthChen · 2026-01-22T14:10:41Z

The pack and unpack logic still remain to be made compatible in the next PR.

codecov-commenter · 2026-01-22T14:45:16Z

Codecov Report

❌ Patch coverage is 66.66667% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.74%. Comparing base (be40746) to head (9f2f062).

Files with missing lines	Patch %	Lines
...ng/http12/message/LengthFieldStreamingDecoder.java	59.45%	13 Missing and 2 partials ⚠️
...oting/websocket/FinalFragmentStreamingDecoder.java	0.00%	3 Missing ⚠️
...he/dubbo/remoting/http12/CompositeInputStream.java	60.00%	1 Missing and 1 partial ⚠️
...ubbo/remoting/http12/message/StreamingDecoder.java	0.00%	1 Missing ⚠️
...apache/dubbo/rpc/protocol/tri/DescriptorUtils.java	0.00%	1 Missing ⚠️
...rotocol/tri/stream/AbstractTripleClientStream.java	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##                3.3   #16041   +/-   ##
=========================================
  Coverage     60.74%   60.74%           
+ Complexity    11754    11745    -9     
=========================================
  Files          1949     1948    -1     
  Lines         88898    88878   -20     
  Branches      13407    13404    -3     
=========================================
- Hits          53998    53986   -12     
  Misses        29337    29337           
+ Partials       5563     5555    -8

Flag	Coverage Δ
integration-tests-java21	`32.26% <59.42%> (+0.03%)`	⬆️
integration-tests-java8	`32.28% <59.42%> (-0.11%)`	⬇️
samples-tests-java21	`32.18% <63.76%> (+0.05%)`	⬆️
samples-tests-java8	`29.84% <63.76%> (+0.15%)`	⬆️
unit-tests-java11	`58.99% <60.86%> (-0.02%)`	⬇️
unit-tests-java17	`58.49% <60.86%> (-0.03%)`	⬇️
unit-tests-java21	`58.50% <60.86%> (-0.01%)`	⬇️
unit-tests-java25	`58.46% <60.86%> (+<0.01%)`	⬆️
unit-tests-java8	`59.02% <60.86%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This pull request unifies the deframing logic for the Triple protocol by replacing the client-side specific TriDecoder with a shared GrpcStreamingDecoder that's already used on the server side. This consolidation eliminates code duplication and improves maintainability.

Changes:

Unified deframing by replacing TriDecoder with GrpcStreamingDecoder for both client and server sides
Changed Stream.Listener and StreamingDecoder.FragmentListener interfaces to accept InputStream instead of byte[] for better streaming support
Added BoundedInputStream wrapper to provide mark/reset support required by deserializers

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/frame/TriDecoder.java	Removed client-specific decoder in favor of unified GrpcStreamingDecoder
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/frame/Deframer.java	Removed interface as StreamingDecoder is now used
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/Stream.java	Updated Listener interface to accept InputStream with messageLength parameter
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/AbstractTripleClientStream.java	Switched to GrpcStreamingDecoder and ByteBufInputStream for data handling
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcStreamingDecoder.java	Updated to return MessageStream with InputStream for better streaming support
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/LengthFieldStreamingDecoder.java	Added BoundedInputStream to provide mark/reset support and proper message boundary handling
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/StreamingDecoder.java	Updated FragmentListener interface to include messageLength parameter
dubbo-common/src/main/java/org/apache/dubbo/rpc/model/PackableMethod.java	Added InputStream-based parseResponse method with default byte[] delegation
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DescriptorUtils.java	Added mark/reset calls to support reading stream twice for method descriptor determination
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/CompositeInputStream.java	Improved error handling in releaseHeadStream with null check and exception handling
dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/frame/TriDecoderTest.java	Removed test for deleted TriDecoder class
dubbo-rpc/dubbo-rpc-triple/src/test/java/org/apache/dubbo/rpc/protocol/tri/stream/MockClientStreamListener.java	Updated to handle InputStream-based messages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T08:11:08Z

...pc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcStreamingDecoder.java

    @Override
-    protected byte[] readRawMessage(InputStream inputStream, int length) throws IOException {
-        byte[] rawMessage = super.readRawMessage(inputStream, length);
-        return compressedFlag ? deCompressedMessage(rawMessage) : rawMessage;
-    }
-
-    private byte[] deCompressedMessage(byte[] rawMessage) {
-        return deCompressor.decompress(rawMessage);
+    protected MessageStream readMessageStream(InputStream inputStream, int length) throws IOException {
+        if (compressedFlag) {
+            // For compressed messages, we need to read bytes first, then decompress
+            byte[] rawMessage = readRawMessage(inputStream, length);
+            byte[] decompressed = deCompressor.decompress(rawMessage);
+            return new MessageStream(new java.io.ByteArrayInputStream(decompressed), decompressed.length);
+        }
+        return super.readMessageStream(inputStream, length);
    }
 }


The refactoring to use GrpcStreamingDecoder (unified deframer) removes the Triple-specific TriDecoder tests without adding equivalent test coverage. While GrpcStreamingDecoder is tested elsewhere, consider adding tests that verify the Triple protocol's specific usage patterns, including: compressed/uncompressed message handling, proper flow control through bytesRead callbacks, and correct integration with the DeCompressor component.

Copilot · 2026-01-23T08:11:08Z

dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DescriptorUtils.java

    public static MethodDescriptor findTripleMethodDescriptor(
            ServiceDescriptor serviceDescriptor, String methodName, InputStream rawMessage) throws IOException {
        MethodDescriptor methodDescriptor = findReflectionMethodDescriptor(serviceDescriptor, methodName);
        if (methodDescriptor == null) {


The call to rawMessage.mark(Integer.MAX_VALUE) assumes the InputStream supports mark/reset and has a sufficient buffer. While BoundedInputStream (used in the unified deframer) extends BufferedInputStream and supports this, consider adding a check or documentation to ensure all callers pass InputStreams that support mark/reset with adequate buffer size. If an InputStream that doesn't support mark/reset is passed, this will fail silently and the subsequent reset() on line 144 will throw an IOException.

Suggested change

if (methodDescriptor == null) {

if (methodDescriptor == null) {

if (!rawMessage.markSupported()) {

throw new IOException("InputStream does not support mark/reset, which is required to resolve overloaded triple methods.");

}

...riple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/AbstractTripleClientStream.java

...tp12/src/main/java/org/apache/dubbo/remoting/http12/message/LengthFieldStreamingDecoder.java

zrlw

LGTM

This commit unifies the deframer implementation between client and server sides for the Triple protocol, eliminating code duplication and improving maintainability. ## Key Changes ### Deleted Files - TriDecoder.java: Removed client-specific deframer implementation - Deframer.java: Removed client deframer interface - RecordListener.java: Removed test helper - TriDecoderTest.java: Removed corresponding test ### Core Modifications 1. **LengthFieldStreamingDecoder**: Added BoundedInputStream to support: - Message boundary isolation (prevents reading beyond current message) - mark/reset support required by Hessian2 and other deserializers - Zero-copy optimization by reading directly from accumulate stream 2. **StreamingDecoder.FragmentListener**: Added messageLength parameter to onFragmentMessage() method for better flow control 3. **AbstractTripleClientStream**: Migrated from TriDecoder to GrpcStreamingDecoder, using ByteBufInputStream to adapt Netty's ByteBuf to InputStream 4. **Stream.Listener.onMessage**: Changed parameter from byte[] to InputStream for unified handling and memory optimization 5. **PackableMethod**: Added default parseResponse(InputStream) method for backward compatibility 6. **CompositeInputStream**: Improved stream release logic to prevent exceptions 7. **DescriptorUtils**: Added mark() call before reading stream to support reset ## Architecture Change Before: - Client: ByteBuf -> TriDecoder (Netty-specific) -> byte[] -> deserializer - Server: InputStream -> LengthFieldStreamingDecoder -> byte[] -> deserializer After: - Client: ByteBuf -> ByteBufInputStream -> GrpcStreamingDecoder -> BoundedInputStream -> deserializer - Server: InputStream -> GrpcStreamingDecoder -> BoundedInputStream -> deserializer ## Bug Fix Fixed BoundedInputStream.reset() not restoring the 'remaining' counter, which caused streams to return EOF after mark/reset. This was the root cause of POJO method invocation failures with "Unexpected serialization type:null" error.

…putStream

zrlw

LGTM

EarthChen force-pushed the refactor-tri-drframer branch 2 times, most recently from 565c117 to 05f0b17 Compare January 23, 2026 07:17

EarthChen requested a review from Copilot January 23, 2026 08:03

Copilot started reviewing on behalf of EarthChen January 23, 2026 08:03 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

EarthChen force-pushed the refactor-tri-drframer branch from ede41a6 to 52cabcf Compare January 23, 2026 09:05

EarthChen mentioned this pull request Jan 24, 2026

Refactor tri pack & unpack use stream #16045

Open

4 tasks

zrlw previously approved these changes Jan 24, 2026

View reviewed changes

zrlw added the status/wait for another approve label Jan 24, 2026

EarthChen dismissed zrlw’s stale review via 3128bd2 January 24, 2026 14:22

EarthChen force-pushed the refactor-tri-drframer branch from 3128bd2 to 39f647d Compare January 24, 2026 14:35

refactor: update tri serialization methods to use InputStream and Out…

9f2f062

…putStream

EarthChen force-pushed the refactor-tri-drframer branch from ccf7d0d to 9f2f062 Compare January 24, 2026 15:04

zrlw approved these changes Jan 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify the deframer of the client and server sides for the Triple protocol #16041

Unify the deframer of the client and server sides for the Triple protocol #16041

EarthChen commented Jan 22, 2026

Uh oh!

EarthChen commented Jan 22, 2026

Uh oh!

codecov-commenter commented Jan 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zrlw left a comment

Uh oh!

zrlw left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Unify the deframer of the client and server sides for the Triple protocol #16041

Are you sure you want to change the base?

Unify the deframer of the client and server sides for the Triple protocol #16041

Conversation

EarthChen commented Jan 22, 2026

What is the purpose of the change?

Checklist

Uh oh!

EarthChen commented Jan 22, 2026

Uh oh!

codecov-commenter commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zrlw left a comment

Choose a reason for hiding this comment

Uh oh!

zrlw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Jan 22, 2026 •

edited

Loading