This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 1ca4559e4b06141b62dbacd75a509ca2cc2e4b9e
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Thu Apr 25 22:46:32 2024 -0700
KUDU-3569 fix race in CFileSet::Iterator::OptimizePKPredicates()
This patch addresses data race reported in KUDU-3569, using
the tablet schema defined by the iterator's projection instead of
the schema stored in the tablet metadata file. The latter might
be swapped by concurrently running AlterTable, which is the root
cause of the data race.
This is a follow-up to 936d7edc4e4b69d2e1f1dffc96760cb3fd57a934.
Change-Id: I92daa74cb86a77a4350f42db9ca5dec3a0d4ff75
Reviewed-on: http://gerrit.cloudera.org:8080/21359
Tested-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
(cherry picked from commit 977f1911fbcc4d5c323d6ae7ce7c1ab100ed11ea)
Conflicts:
src/kudu/tablet/cfile_set.h
Reviewed-on: http://gerrit.cloudera.org:8080/21366
---
src/kudu/tablet/cfile_set.cc | 21 ++++++++++++++++-----
src/kudu/tablet/cfile_set.h | 1 -
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/kudu/tablet/cfile_set.cc b/src/kudu/tablet/cfile_set.cc
index 06d5f46e9..85803e6ac 100644
--- a/src/kudu/tablet/cfile_set.cc
+++ b/src/kudu/tablet/cfile_set.cc
@@ -431,18 +431,29 @@ Status CFileSet::Iterator::OptimizePKPredicates(ScanSpec*
spec) {
EncodedKey* implicit_ub_key = nullptr;
bool modify_lower_bound_key = false;
bool modify_upper_bound_key = false;
- const Schema& tablet_schema = *base_data_->tablet_schema();
+
+ // Keep a reference to the current tablet schema to use in this scope. That's
+ // preventing data races when a concurrently running Tablet::AlterSchema()
+ // is destroying the underlying Schema object in TabletMetadata::SetSchema().
+ // Since the only information required from the schema in this context
+ // is primary key-related, any snapshot of the tablet's schema is good enough
+ // since primary key information is immutable for an existing Kudu table.
+ //
+ // NOTE: it's not possible to use the projection returned by
+ // CFileSet::Iterator::schema() because it might not contain
information
+ // on primary key columns in case of non-FT scans.
+ SchemaPtr tablet_schema(base_data_->tablet_schema());
if (!lb_key || lb_key->encoded_key() < base_data_->min_encoded_key_) {
RETURN_NOT_OK(EncodedKey::DecodeEncodedString(
- tablet_schema, &arena_, base_data_->min_encoded_key_,
&implicit_lb_key));
+ *tablet_schema, &arena_, base_data_->min_encoded_key_,
&implicit_lb_key));
spec->SetLowerBoundKey(implicit_lb_key);
modify_lower_bound_key = true;
}
RETURN_NOT_OK(EncodedKey::DecodeEncodedString(
- tablet_schema, &arena_, base_data_->max_encoded_key_, &implicit_ub_key));
- Status s = EncodedKey::IncrementEncodedKey(tablet_schema, &implicit_ub_key,
&arena_);
+ *tablet_schema, &arena_, base_data_->max_encoded_key_,
&implicit_ub_key));
+ Status s = EncodedKey::IncrementEncodedKey(*tablet_schema, &implicit_ub_key,
&arena_);
// Reset the exclusive_upper_bound_key only when we can get a valid and
smaller upper bound key.
// In the case IncrementEncodedKey return ERROR status due to allocation
fails or no
// lexicographically greater key exists, we fall back to scan the rowset
without optimizing the
@@ -453,7 +464,7 @@ Status CFileSet::Iterator::OptimizePKPredicates(ScanSpec*
spec) {
}
if (modify_lower_bound_key || modify_upper_bound_key) {
- spec->OptimizeScan(tablet_schema, &arena_, true);
+ spec->OptimizeScan(*tablet_schema, &arena_, true);
}
return Status::OK();
}
diff --git a/src/kudu/tablet/cfile_set.h b/src/kudu/tablet/cfile_set.h
index 7a119c4c6..5aea63033 100644
--- a/src/kudu/tablet/cfile_set.h
+++ b/src/kudu/tablet/cfile_set.h
@@ -35,7 +35,6 @@
#include "kudu/common/schema.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/map-util.h"
-#include "kudu/gutil/port.h"
#include "kudu/tablet/rowset_metadata.h"
#include "kudu/util/make_shared.h"
#include "kudu/util/memory/arena.h"
