Skip to content

Commit 788434a

Browse files
committed
[yugabyte#18771, yugabyte#21352] docdb: Fix LightweightMessage max size when parsing
Summary: LightweightMessage currently sets a maximum size for reading to `rpc_max_message_size` (default 255 MB), but Preparer batches based on `protobuf_message_total_bytes_limit` (default 511 MB). This results in cases where under default settings, we may have protobufs between 255 MB and 511 MB, which `LightweightMessage::ParseFromSlice` is unable to read. This diff changes the limit to use `protobuf_message_total_bytes_limit`, so that protobufs between `rpc_max_message_size` and `protobuf_message_total_bytes_limit` can be parsed properly. This addresses errors such as: ``` Found a corruption in a closed log segment: OK Error: Corruption (yb/consensus/log_util.cc:965): Log file corruption detected.: Failed to parse PB at offset: 26013423, length: 303149529. Cause: Corruption (yb/rpc/lightweight_message.cc:376): Failed to parse ‘entry’: Failed trying to read batch #5 at offset 26013423 for log segment /mnt/d0/yb-data/tserver/wals/table-12345678901234567890123456789012/tablet-12345678901234567890123456789012/wal-000003000: ... ``` (length larger than 255 MB) when such protobufs are written to WALs. This also fixes cause of flakiness for TabletPeerTest.MaxRaftBatchProtobufLimit in TSAN builds. Jira: DB-7654, DB-10251 Test Plan: Jenkins. Added test: ``` yb_build.sh --cxx-test rpc_lwproto-test --gtest_filter LWProtoTest.BigMessage ``` Also ran TabletPeerTest.MaxRaftBatchProtobufLimit 100x on Jenkins. Reviewers: sergei, qhu Reviewed By: qhu Subscribers: yyan, bogdan, rthallam, ybase Differential Revision: https://phorge.dev.yugabyte.com/D33041
1 parent 7b1f22a commit 788434a

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

src/yb/rpc/lightweight_message.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ DEFINE_UNKNOWN_uint64(rpc_max_message_size, 255_MB,
3131
"The maximum size of a message of any RPC that the server will accept. The sum of "
3232
"consensus_max_batch_size_bytes and 1KB should be less than rpc_max_message_size");
3333

34+
DECLARE_int32(protobuf_message_total_bytes_limit);
35+
3436
using google::protobuf::internal::WireFormatLite;
3537
using google::protobuf::io::CodedOutputStream;
3638

@@ -378,8 +380,7 @@ Status ParseFailed(const char* field_name) {
378380
}
379381

380382
void SetupLimit(google::protobuf::io::CodedInputStream* in) {
381-
in->SetTotalBytesLimit(narrow_cast<int>(FLAGS_rpc_max_message_size),
382-
narrow_cast<int>(FLAGS_rpc_max_message_size * 3 / 4));
383+
in->SetTotalBytesLimit(FLAGS_protobuf_message_total_bytes_limit, 0 /* unused */);
383384
}
384385

385386
ThreadSafeArena& empty_arena() {

src/yb/rpc/lwproto-test.cc

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,33 @@
1919

2020
#include "yb/util/faststring.h"
2121
#include "yb/util/logging.h"
22+
#include "yb/util/random_util.h"
23+
#include "yb/util/size_literals.h"
2224
#include "yb/util/test_macros.h"
2325

26+
DECLARE_int32(protobuf_message_total_bytes_limit);
27+
DECLARE_uint64(rpc_max_message_size);
28+
2429
namespace yb {
2530
namespace rpc {
2631

32+
namespace {
33+
34+
template <typename PB>
35+
Status SerializePB(PB& pb, faststring& buf) {
36+
LOG(INFO) << "Source proto: " << pb.ShortDebugString();
37+
38+
AnyMessageConstPtr ptr(&pb);
39+
buf.resize(ptr.SerializedSize());
40+
RETURN_NOT_OK(ptr.SerializeToArray(buf.data()));
41+
42+
LOG(INFO) << "Binary dump: " << Slice(buf).ToDebugHexString();
43+
44+
return Status::OK();
45+
}
46+
47+
} // namespace
48+
2749
// Make sure LW protobuf skips unknown fields.
2850
TEST(LWProtoTest, SkipsUnknownFields) {
2951
rpc_test::TestObjectPB pb;
@@ -37,13 +59,7 @@ TEST(LWProtoTest, SkipsUnknownFields) {
3759
pb.set_int32_2(15);
3860
pb.mutable_record2()->set_text("record2");
3961

40-
LOG(INFO) << "Source proto: " << pb.ShortDebugString();
41-
42-
AnyMessageConstPtr ptr(&pb);
43-
buf.resize(ptr.SerializedSize());
44-
ASSERT_OK(ptr.SerializeToArray(buf.data()));
45-
46-
LOG(INFO) << "Binary dump: " << Slice(buf).ToDebugHexString();
62+
ASSERT_OK(SerializePB(pb, buf));
4763
}
4864

4965
{
@@ -82,5 +98,29 @@ TEST(LWProtoTest, SkipsUnknownFields) {
8298
}
8399
}
84100

101+
// Test a very large proto (rpc_max_message_size < proto size < protobuf_message_total_bytes_limit).
102+
TEST(LWProtoTest, BigMessage) {
103+
faststring buf;
104+
rpc_test::TestObjectPB pb;
105+
106+
ANNOTATE_UNPROTECTED_WRITE(FLAGS_rpc_max_message_size) = 4_MB;
107+
ANNOTATE_UNPROTECTED_WRITE(FLAGS_protobuf_message_total_bytes_limit) = 8_MB;
108+
109+
constexpr auto kPBSize = 6_MB;
110+
111+
pb.set_string1(RandomHumanReadableString(kPBSize));
112+
ASSERT_OK(SerializePB(pb, buf));
113+
114+
ThreadSafeArena arena;
115+
rpc_test::LWTestObjectPBv2 lwpb2(&arena);
116+
AnyMessagePtr ptr(&lwpb2);
117+
118+
ASSERT_OK(ptr.ParseFromSlice(Slice(buf)));
119+
LOG(INFO) << "Read lightweight proto: " << lwpb2.ShortDebugString();
120+
121+
ASSERT_TRUE(lwpb2.has_string1());
122+
ASSERT_EQ(pb.string1(), lwpb2.string1());
123+
}
124+
85125
} // namespace rpc
86126
} // namespace yb

0 commit comments

Comments
 (0)