Skip to content

Commit

Permalink
c-deps: fix an infinite loop in MVCCScan
Browse files Browse the repository at this point in the history
MVCCScan would previously loop forever if asked to scan more than 2GB of
data. The root problem was in the calculation of the next buffer size to
use in `chunkedBuffer::put` which was not accounting for an `int` being
a signed 32-bit value. Changed the logic in `chunkedBuffer::put` to use
a maximum buffer size of 128 MB to avoid hitting this problem and avoid
wasting excessive space wasteage if only a tiny bit of additional space
is needed.

Fixes #32896

Release note (bug fix): Fix an infinite loop in a low-level scanning
routine that could be hit in unusual circumstances.
  • Loading branch information
petermattis committed Dec 12, 2018
1 parent 67e4d98 commit 153fcf3
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 3 deletions.
1 change: 1 addition & 0 deletions c-deps/libroach/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ enable_testing()
# List of tests to build and run. Tests in `ccl/` are linked against roachccl, all others
# are linked against roach only.
set(tests
chunked_buffer_test.cc
db_test.cc
encoding_test.cc
file_registry_test.cc
Expand Down
14 changes: 11 additions & 3 deletions c-deps/libroach/chunked_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ void chunkedBuffer::Clear() {
// indicate that the required size of this buffer will soon be
// len+next_size_hint, to prevent excessive resize operations.
void chunkedBuffer::put(const char* data, int len, int next_size_hint) {
const int avail = bufs_.empty() ? 0 : (bufs_.back().len - (buf_ptr_ - bufs_.back().data));
if (len > avail) {
for (;;) {
const size_t avail = bufs_.empty() ? 0 : (bufs_.back().len - (buf_ptr_ - bufs_.back().data));
if (len <= avail) {
break;
}

// If it's bigger than the last buf's capacity, we fill the last buf,
// allocate a new one, and write the remainder to the new one. Our new
// buf's size will be the next power of two past the size of the last buf
Expand All @@ -59,8 +63,12 @@ void chunkedBuffer::put(const char* data, int len, int next_size_hint) {
data += avail;
len -= avail;

const int max_size = 128 << 20; // 128 MB
int new_size = bufs_.empty() ? 16 : bufs_.back().len * 2;
for (; new_size < len + next_size_hint; new_size *= 2) {
for (; new_size < len + next_size_hint && new_size < max_size; new_size *= 2) {
}
if (new_size > max_size) {
new_size = max_size;
}

DBSlice new_buf;
Expand Down
40 changes: 40 additions & 0 deletions c-deps/libroach/chunked_buffer_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2018 The Cockroach Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
// implied. See the License for the specific language governing
// permissions and limitations under the License.

#include <gtest/gtest.h>
#include "chunked_buffer.h"

// Verify we can a chunked buffer can hold more than 2GB of data when
// writing in pieces that are smaller than the maximum chunk size (128
// MB). See #32896.
TEST(ChunkedBuffer, PutSmall) {
const std::string data(1 << 20, '.'); // 1 MB
const int64_t total = 3LL << 30; // 3 GB
cockroach::chunkedBuffer buf;
for (int64_t sum = 0; sum < total; sum += data.size()) {
buf.Put(data, data);
}
}

// Verify we can a chunked buffer can hold more than 2GB of data when
// writing in pieces that are larger than the maximum chunk size (128
// MB). See #32896.
TEST(ChunkedBuffer, PutLarge) {
const std::string data(256 << 20, '.'); // 256 MB
const int64_t total = 3LL << 30; // 3 GB
cockroach::chunkedBuffer buf;
for (int64_t sum = 0; sum < total; sum += data.size()) {
buf.Put(data, data);
}
}

0 comments on commit 153fcf3

Please sign in to comment.