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 75fc23a988fb582e5d8ffcdd0289bdd6b637a1e6 Author: Alexey Serbin <[email protected]> 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 <[email protected]> Reviewed-by: Abhishek Chennaka <[email protected]> (cherry picked from commit 3912a97cd8998ef04c4e6f9c38bd365c582e8171) Reviewed-on: http://gerrit.cloudera.org:8080/21365 Reviewed-by: Wang Xixu <[email protected]> --- 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 d0f985f87..268ff4d8f 100644 --- a/src/kudu/tablet/diskrowset.cc +++ b/src/kudu/tablet/diskrowset.cc @@ -578,10 +578,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)); @@ -627,24 +636,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 3cb68806f..ff4fd1958 100644 --- a/src/kudu/tablet/diskrowset.h +++ b/src/kudu/tablet/diskrowset.h @@ -494,9 +494,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.
