kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jdcry...@apache.org
Subject [1/3] incubator-kudu git commit: Don't take a reference to a temporary when comparing first and last encoded keys
Date Mon, 22 Feb 2016 23:45:15 GMT
Repository: incubator-kudu
Updated Branches:
  refs/heads/master d43ec1e9d -> 2878c5862

Don't take a reference to a temporary when comparing first and last encoded keys

In DiskRowSeWriter we have a check that makes sure that the first encoded key is less
than or equal to the last key. We perform the check by creating slices for the keys
and comparing the slices, for 'first_enc_slice' we're building it with a reference
to the string returned by key_index_writer()->GetMetaValueOrDie(), which ceases to
exist immediately. This because in libc++, returning the std::string copy is actually
a new copy of the underlying data, so our Slice points to a temporary bit of memory.

By making sure that the string survives the check the crash disappears.

While this doesn't contain a test for the crash it was easily reproducible by setting
kFlushDueToTimeMs in tablet_peer_mm_ops.cc to something like 2 and then running any
test that performed a reasonable amount of flushes (like create-table-itest). With
this fix it isn't reproducible anymore.

I also tested this with a large-ish table (150GBs) which would crash semi-regularly
and can't observe the crashes anymore.

Change-Id: Iaa1cbb087c6467bb5da777234270df089a4b3252
Reviewed-on: http://gerrit.cloudera.org:8080/2271
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>

Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/dbc1de3f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/dbc1de3f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/dbc1de3f

Branch: refs/heads/master
Commit: dbc1de3f5346f14fddf4b5d99f7ce381075a31f9
Parents: d43ec1e
Author: David Alves <david.alves@cloudera.com>
Authored: Sat Feb 20 18:17:58 2016 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Sun Feb 21 23:16:20 2016 +0000

 src/kudu/tablet/diskrowset.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc
index 55decd8..3e28500 100644
--- a/src/kudu/tablet/diskrowset.cc
+++ b/src/kudu/tablet/diskrowset.cc
@@ -213,7 +213,10 @@ Status DiskRowSetWriter::FinishAndReleaseBlocks(ScopedWritableBlockCloser*
   // Save the last encoded (max) key
   CHECK_GT(last_encoded_key_.size(), 0);
   Slice last_enc_slice(last_encoded_key_);
-  Slice first_enc_slice(key_index_writer()->GetMetaValueOrDie(DiskRowSet::kMinKeyMetaEntryName));
+  std::string first_encoded_key =
+      key_index_writer()->GetMetaValueOrDie(DiskRowSet::kMinKeyMetaEntryName);
+  Slice first_enc_slice(first_encoded_key);
   CHECK_LE(first_enc_slice.compare(last_enc_slice), 0)
       << "First Key not <= Last key: first_key=" << first_enc_slice.ToDebugString()
       << "   last_key=" << last_enc_slice.ToDebugString();

View raw message