THRIFT-5863: Make TServerTransport able to customize the max message size

[dengzhhu653]

…Transport

• Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[Jens-G]

Question: Why is it considered good to have multiple configuration instances, in opposite to use a shared instance through the whole transport/protocol stack? What's the added value exactly?

[dengzhhu653]

the main idea is to avoid another new constructor in TSocket, the deepCopy will copy the threshold from the configuration_ into socket.getConfiguration(), the original socket.getConfiguration() is still shared across the whole transport/protocol stack.

[dengzhhu653]

Add a new constructor in TSocket, this constructor might also help the client create the instance over the SSLSocket. Thank you for the comment!

[fishy]

I think in general we want to propagate a single TConfiguration instance across the stack, vs. copying. This is at least implied in the spec of TConfiguration (https://github.com/apache/thrift/blob/master/doc/specs/thrift-tconfiguration.md, it mentioned "...we want to pass the instance by reference rather than by value.").

That's also how I implemented it in go. see the NOTE part of the doc of TConfiguration for some explanation: https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TConfiguration

When implementing it for go we also deprecated most of the old constructors doing configuration without TConfiguration.

[dengzhhu653]

Hi @fishy, I'm confused here, do you mean this line:

thrift/lib/java/src/main/java/org/apache/thrift/transport/TServerTransport.java

Line 62 in aa5f183

configuration.deepCopy(configuration_);

I create the new TConfiguration for the compatibility of maxFrameSize(int maxFrameSize) method(for pushing the maxFrameSize to the TConfiguration directly), and this calls at the service startup, there is no transport pipeline built successfully yet.
After the server accepting the connection, the transport pipeline should reuse the TSocket's configuration, the PR is saying the same story per my understanding, as I don't change how to propagate the TConfiguration through the stack.

To make the propagation and validation clear, I raised another PR #3127, for a stack like: TSocket -> TSaslTransport(TMemoryInputTransport) -> TUGIAssumingTransport
Before:
1, TSocket, TSaslTransport and TMemoryInputTransport, have a local reference to the same TConfiguration.
2, TSocket, TSaslTransport and TMemoryInputTransport have it own private knownMessageSize and remainingMessageSize, updated and checked separately.
After:
1, TSaslTransport and TMemoryInputTransport don't have such local reference, while TSaslTransport can use getConfiguration to retrieve the TConfiguration from end TSocket, same to TUGIAssumingTransport, which ensures the same TConfiguration is shared across the stack.
2, Only TSocket updates and checks the message size.

[dengzhhu653]

Hi @Jens-G @fishy, any idea about this PR? this gives the server a chance to change the max message/frame size of the accepted transport, and it doesn't block anything per my understanding.