This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 3912a97cd KUDU-3570 fix use-after-free in MajorDeltaCompactionOp
3912a97cd is described below
commit 3912a97cd8998ef04c4e6f9c38bd365c582e8171
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Fri Apr 26 16:55:37 2024 -0700
KUDU-3570 fix use-after-free in MajorDeltaCompactionOp
This patch addresses heap-use-after-free and data race issues reported
in KUDU-3570. With this and one prior patch, neither TSAN nor ASAN
reports any warnings when running alter_table-randomized-test, at least
that's the stats collected from more than 100 iterations.
Change-Id: I491c6d98bed8780bcfb62f152db471d7a260d305
Reviewed-on: http://gerrit.cloudera.org:8080/21362
Tested-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
---
src/kudu/tablet/diskrowset.cc | 22 +++++++++++++---------
src/kudu/tablet/diskrowset.h | 3 ++-
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc
index f6c92c9ed..47b616fa4 100644
--- a/src/kudu/tablet/diskrowset.cc
+++ b/src/kudu/tablet/diskrowset.cc
@@ -580,10 +580,19 @@ Status
DiskRowSet::MajorCompactDeltaStoresWithColumnIds(const vector<ColumnId>&
std::lock_guard<Mutex> l(*mutable_delta_tracker()->compact_flush_lock());
RETURN_NOT_OK(mutable_delta_tracker()->CheckWritableUnlocked());
+ // Keep a reference to the tablet's schema. This is to prevent race condition
+ // when concurrently running Tablet::AlterSchema() destroys the underlying
+ // Schema object by calling TabletMetadata::SetSchema().
+ //
// TODO(todd): do we need to lock schema or anything here?
+ SchemaPtr schema_ptr = rowset_metadata_->tablet_schema();
+
+ RowIteratorOptions opts;
+ opts.projection = schema_ptr.get();
+ opts.io_context = io_context;
unique_ptr<MajorDeltaCompaction> compaction;
- RETURN_NOT_OK(NewMajorDeltaCompaction(col_ids, std::move(history_gc_opts),
- io_context, &compaction));
+ RETURN_NOT_OK(NewMajorDeltaCompaction(
+ col_ids, opts, std::move(history_gc_opts), &compaction));
RETURN_NOT_OK(compaction->Compact(io_context));
@@ -629,24 +638,19 @@ Status
DiskRowSet::MajorCompactDeltaStoresWithColumnIds(const vector<ColumnId>&
}
Status DiskRowSet::NewMajorDeltaCompaction(const vector<ColumnId>& col_ids,
+ const RowIteratorOptions& opts,
HistoryGcOpts history_gc_opts,
- const IOContext* io_context,
unique_ptr<MajorDeltaCompaction>*
out) const {
DCHECK(open_);
shared_lock<rw_spinlock> l(component_lock_);
- const SchemaPtr schema_ptr = rowset_metadata_->tablet_schema();
-
- RowIteratorOptions opts;
- opts.projection = schema_ptr.get();
- opts.io_context = io_context;
vector<shared_ptr<DeltaStore>> included_stores;
unique_ptr<DeltaIterator> delta_iter;
RETURN_NOT_OK(delta_tracker_->NewDeltaFileIterator(
opts, REDO, &included_stores, &delta_iter));
out->reset(new MajorDeltaCompaction(rowset_metadata_->fs_manager(),
- *schema_ptr,
+ *opts.projection,
base_data_.get(),
std::move(delta_iter),
std::move(included_stores),
diff --git a/src/kudu/tablet/diskrowset.h b/src/kudu/tablet/diskrowset.h
index ea1250e3e..a6b75ee6f 100644
--- a/src/kudu/tablet/diskrowset.h
+++ b/src/kudu/tablet/diskrowset.h
@@ -500,9 +500,10 @@ class DiskRowSet :
Status Open(const fs::IOContext* io_context);
// Create a new major delta compaction object to compact the specified
columns.
+ // TODO(aserbin): use the move semantics for RowIteratorOptions
Status NewMajorDeltaCompaction(const std::vector<ColumnId>& col_ids,
+ const RowIteratorOptions& opts,
HistoryGcOpts history_gc_opts,
- const fs::IOContext* io_context,
std::unique_ptr<MajorDeltaCompaction>* out)
const;
// Major compacts all the delta files for the specified columns.
