[
https://issues.apache.org/jira/browse/THRIFT-6183?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6183:
-------------------------------
Description:
TNonblockingServer caps the frame it will accept from a connection at its own
MAX_FRAME_SIZE (lib/cpp/src/thrift/server/TNonblockingServer.h:127), which has
been 256 * 1024 * 1024 since THRIFT-1337 landed it in 2011 - years before
TConfiguration existed. The library-wide default is
TConfiguration::DEFAULT_MAX_FRAME_SIZE = 16384000, whose own comment reads
"this value is used consistently across all Thrift libraries".
The two disagree by a factor of 16:
{code}
TNonblockingServer::getMaxFrameSize() 268435456
TConfiguration::DEFAULT_MAX_FRAME_SIZE 16384000
{code}
TConfiguration appears zero times in TNonblockingServer.h and
TNonblockingServer.cpp, so the server neither reads a configured value nor
inherits the default. It starts from its own constant, and only
setMaxFrameSize() moves it.
That constant is what sizes the read buffer. TConnection::transition() resets
the read buffer to the frame length the peer declared before any payload byte
has arrived, so a four-byte length prefix at the server's own maximum decides
the allocation. Measured on a Linux x86-64 build, one connection sending
nothing but that prefix takes the process from a VmPeak of 145,880 kB to
604,636 kB. MAX_CONNECTIONS defaults to INT_MAX.
Other bindings do not have this gap. Java's AbstractNonblockingServer takes its
per-frame limit from trans_.getMaxFrameSize(), that is from TConfiguration, and
keeps a separate aggregate read-buffer budget on top of it.
The change points TNonblockingServer::MAX_FRAME_SIZE at
TConfiguration::DEFAULT_MAX_FRAME_SIZE. The constant is private, so this is not
an API change, and setMaxFrameSize() still overrides it.
Compatibility: this lowers a shipped default. A deployment that accepts frames
between 16,384,000 and 268,435,456 bytes on TNonblockingServer today and does
not call setMaxFrameSize() will start closing those connections. Needs a
release note.
Out of scope here, both worth their own tickets: TNonblockingServer still does
not consult TConfiguration at all, which is a larger design change; and
lib/cpp/src/thrift/transport/TBufferTransports.h:349 still carries a 256 MB
TFramedTransport::DEFAULT_MAX_FRAME_SIZE that nothing reads any more, which is
public API and is left alone here.
was:
TNonblockingServer caps the frame it will accept from a connection at its own
MAX_FRAME_SIZE (lib/cpp/src/thrift/server/TNonblockingServer.h:127), which has
been 256 * 1024 * 1024 since THRIFT-1337 landed it in 2011 - years before
TConfiguration existed. The library-wide default is
TConfiguration::DEFAULT_MAX_FRAME_SIZE = 16384000, whose own comment reads
"this value is used consistently across all Thrift libraries".
The two disagree by a factor of 16:
TNonblockingServer::getMaxFrameSize() 268435456
TConfiguration::DEFAULT_MAX_FRAME_SIZE 16384000
TConfiguration appears zero times in TNonblockingServer.h and
TNonblockingServer.cpp, so the server neither reads a configured value nor
inherits the default. It starts from its own constant, and only
setMaxFrameSize() moves it.
That constant is what sizes the read buffer. TConnection::transition() resets
the read buffer to the frame length the peer declared before any payload byte
has arrived, so a four-byte length prefix at the server's own maximum decides
the allocation. Measured on a Linux x86-64 build, one connection sending
nothing but that prefix takes the process from a VmPeak of 145,880 kB to
604,636 kB. MAX_CONNECTIONS defaults to INT_MAX.
Other bindings do not have this gap. Java's AbstractNonblockingServer takes its
per-frame limit from trans_.getMaxFrameSize(), that is from TConfiguration, and
keeps a separate aggregate read-buffer budget on top of it.
The change points TNonblockingServer::MAX_FRAME_SIZE at
TConfiguration::DEFAULT_MAX_FRAME_SIZE. The constant is private, so this is not
an API change, and setMaxFrameSize() still overrides it.
Compatibility: this lowers a shipped default. A deployment that accepts frames
between 16,384,000 and 268,435,456 bytes on TNonblockingServer today and does
not call setMaxFrameSize() will start closing those connections. Needs a
release note.
Out of scope here, both worth their own tickets: TNonblockingServer still does
not consult TConfiguration at all, which is a larger design change; and
lib/cpp/src/thrift/transport/TBufferTransports.h:349 still carries a 256 MB
TFramedTransport::DEFAULT_MAX_FRAME_SIZE that nothing reads any more, which is
public API and is left alone here.
> Use the library-wide default frame size in TNonblockingServer in the C++
> library
> --------------------------------------------------------------------------------
>
> Key: THRIFT-6183
> URL: https://issues.apache.org/jira/browse/THRIFT-6183
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Reporter: Jens Geyer
> Priority: Major
> Labels: Breaking-Change
> Time Spent: 10m
> Remaining Estimate: 0h
>
> TNonblockingServer caps the frame it will accept from a connection at its own
> MAX_FRAME_SIZE (lib/cpp/src/thrift/server/TNonblockingServer.h:127), which
> has been 256 * 1024 * 1024 since THRIFT-1337 landed it in 2011 - years before
> TConfiguration existed. The library-wide default is
> TConfiguration::DEFAULT_MAX_FRAME_SIZE = 16384000, whose own comment reads
> "this value is used consistently across all Thrift libraries".
> The two disagree by a factor of 16:
> {code}
> TNonblockingServer::getMaxFrameSize() 268435456
> TConfiguration::DEFAULT_MAX_FRAME_SIZE 16384000
> {code}
> TConfiguration appears zero times in TNonblockingServer.h and
> TNonblockingServer.cpp, so the server neither reads a configured value nor
> inherits the default. It starts from its own constant, and only
> setMaxFrameSize() moves it.
> That constant is what sizes the read buffer. TConnection::transition() resets
> the read buffer to the frame length the peer declared before any payload byte
> has arrived, so a four-byte length prefix at the server's own maximum decides
> the allocation. Measured on a Linux x86-64 build, one connection sending
> nothing but that prefix takes the process from a VmPeak of 145,880 kB to
> 604,636 kB. MAX_CONNECTIONS defaults to INT_MAX.
> Other bindings do not have this gap. Java's AbstractNonblockingServer takes
> its per-frame limit from trans_.getMaxFrameSize(), that is from
> TConfiguration, and keeps a separate aggregate read-buffer budget on top of
> it.
> The change points TNonblockingServer::MAX_FRAME_SIZE at
> TConfiguration::DEFAULT_MAX_FRAME_SIZE. The constant is private, so this is
> not an API change, and setMaxFrameSize() still overrides it.
> Compatibility: this lowers a shipped default. A deployment that accepts
> frames between 16,384,000 and 268,435,456 bytes on TNonblockingServer today
> and does not call setMaxFrameSize() will start closing those connections.
> Needs a release note.
> Out of scope here, both worth their own tickets: TNonblockingServer still
> does not consult TConfiguration at all, which is a larger design change; and
> lib/cpp/src/thrift/transport/TBufferTransports.h:349 still carries a 256 MB
> TFramedTransport::DEFAULT_MAX_FRAME_SIZE that nothing reads any more, which
> is public API and is left alone here.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
