Skip to content

Commit

Permalink
Rename is_backfilling to retain_delete_markers
Browse files Browse the repository at this point in the history
Summary:

For `Schema` and `TableProperties`, rename `is_backfilling_` to
`retain_delete_markers_` (and similarly for related getter/setter
functions).  This is clearer because it is set when the index gets
created all the way to the end of backfill.  In other words, it is also
set during the `DELETE_ONLY` and `WRITE_AND_DELETE` permissions _before_
backfill.

Furthermore, this helps disambiguate with the other existing
`TableInfo::is_backfilling_`.

Test Plan:

`./yb_build.sh --cxx-test common_schema-test`

Reviewers: amitanand

Reviewed By: amitanand

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D9636
  • Loading branch information
jaki committed Oct 16, 2020
1 parent 765aa1b commit 9f1733f
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/yb/common/common.proto
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ message TablePropertiesPB {
optional bool use_mangled_column_name = 6 [ default = false ];
optional int32 num_tablets = 7 [ default = 0 ];
optional bool is_ysql_catalog_table = 8 [ default = false ];
optional bool is_backfilling = 9 [ default = false ];
optional bool retain_delete_markers = 9 [ default = false ];
optional uint64 backfilling_timestamp = 10;
}

Expand Down
8 changes: 4 additions & 4 deletions src/yb/common/schema-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ TEST(TestSchema, TestSchema) {
"consistency_level: STRONG "
"use_mangled_column_name: false "
"is_ysql_catalog_table: false "
"is_backfilling: false",
"retain_delete_markers: false",
schema.ToString());
EXPECT_EQ("key[string NOT NULL NOT A PARTITION KEY]", schema.column(0).ToString());
EXPECT_EQ("uint32 NULLABLE NOT A PARTITION KEY", schema.column(1).TypeToString());
Expand Down Expand Up @@ -373,7 +373,7 @@ TEST(TestSchema, TestCreateProjection) {
"consistency_level: STRONG "
"use_mangled_column_name: false "
"is_ysql_catalog_table: false "
"is_backfilling: false",
"retain_delete_markers: false",
partial_schema.ToString());

// By names, with IDS
Expand All @@ -386,7 +386,7 @@ TEST(TestSchema, TestCreateProjection) {
"consistency_level: STRONG "
"use_mangled_column_name: false "
"is_ysql_catalog_table: false "
"is_backfilling: false",
"retain_delete_markers: false",
schema_with_ids.column_id(0),
schema_with_ids.column_id(1),
schema_with_ids.column_id(3)),
Expand All @@ -410,7 +410,7 @@ TEST(TestSchema, TestCreateProjection) {
"consistency_level: STRONG "
"use_mangled_column_name: false "
"is_ysql_catalog_table: false "
"is_backfilling: false",
"retain_delete_markers: false",
schema_with_ids.column_id(0),
schema_with_ids.column_id(1),
schema_with_ids.column_id(3)),
Expand Down
12 changes: 6 additions & 6 deletions src/yb/common/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void TableProperties::ToTablePropertiesPB(TablePropertiesPB *pb) const {
pb->set_num_tablets(num_tablets_);
}
pb->set_is_ysql_catalog_table(is_ysql_catalog_table_);
pb->set_is_backfilling(is_backfilling_);
pb->set_retain_delete_markers(retain_delete_markers_);
}

TableProperties TableProperties::FromTablePropertiesPB(const TablePropertiesPB& pb) {
Expand Down Expand Up @@ -125,8 +125,8 @@ TableProperties TableProperties::FromTablePropertiesPB(const TablePropertiesPB&
if (pb.has_is_ysql_catalog_table()) {
table_properties.set_is_ysql_catalog_table(pb.is_ysql_catalog_table());
}
if (pb.has_is_backfilling()) {
table_properties.SetIsBackfilling(pb.is_backfilling());
if (pb.has_retain_delete_markers()) {
table_properties.SetRetainDeleteMarkers(pb.retain_delete_markers());
}
return table_properties;
}
Expand All @@ -153,8 +153,8 @@ void TableProperties::AlterFromTablePropertiesPB(const TablePropertiesPB& pb) {
if (pb.has_is_ysql_catalog_table()) {
set_is_ysql_catalog_table(pb.is_ysql_catalog_table());
}
if (pb.has_is_backfilling()) {
SetIsBackfilling(pb.is_backfilling());
if (pb.has_retain_delete_markers()) {
SetRetainDeleteMarkers(pb.retain_delete_markers());
}
}

Expand All @@ -167,7 +167,7 @@ void TableProperties::Reset() {
use_mangled_column_name_ = false;
num_tablets_ = 0;
is_ysql_catalog_table_ = false;
is_backfilling_ = false;
retain_delete_markers_ = false;
}

string TableProperties::ToString() const {
Expand Down
22 changes: 14 additions & 8 deletions src/yb/common/schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ class TableProperties {
contain_counters_ == other.contain_counters_;

// Ignoring num_tablets_.
// Ignoring is_backfilling_.
// Ignoring retain_delete_markers_.
// Ignoring wal_retention_secs_.
}

Expand Down Expand Up @@ -412,7 +412,7 @@ class TableProperties {
// Ignoring num_tablets_.
// Ignoring use_mangled_column_name_.
// Ignoring contain_counters_.
// Ignoring is_backfilling_.
// Ignoring retain_delete_markers_.
// Ignoring wal_retention_secs_.
return true;
}
Expand Down Expand Up @@ -493,9 +493,13 @@ class TableProperties {
return is_ysql_catalog_table_;
}

bool IsBackfilling() const { return is_backfilling_; }
bool retain_delete_markers() const {
return retain_delete_markers_;
}

void SetIsBackfilling(bool is_backfilling) { is_backfilling_ = is_backfilling; }
void SetRetainDeleteMarkers(bool retain_delete_markers) {
retain_delete_markers_ = retain_delete_markers;
}

void ToTablePropertiesPB(TablePropertiesPB *pb) const;

Expand All @@ -516,7 +520,7 @@ class TableProperties {
int64_t default_time_to_live_ = kNoDefaultTtl;
bool contain_counters_ = false;
bool is_transactional_ = false;
bool is_backfilling_ = false;
bool retain_delete_markers_ = false;
YBConsistencyLevel consistency_level_ = YBConsistencyLevel::STRONG;
TableId copartition_table_id_ = kNoCopartitionTableId;
boost::optional<uint32_t> wal_retention_secs_;
Expand Down Expand Up @@ -729,7 +733,9 @@ class Schema {
table_properties_.SetTransactional(is_transactional);
}

void SetIsBackfilling(bool is_backfilling) { table_properties_.SetIsBackfilling(is_backfilling); }
void SetRetainDeleteMarkers(bool retain_delete_markers) {
table_properties_.SetRetainDeleteMarkers(retain_delete_markers);
}

// Return the column index corresponding to the given column,
// or kColumnNotFound if the column is not in this schema.
Expand Down Expand Up @@ -1007,8 +1013,8 @@ class Schema {

// Return true if the schemas have exactly the same set of columns
// and respective types, and equivalent properties.
// For example, one table property could have a different properties like wal_retention_secs_
// is_backfilling_ but still be equivalent.
// For example, one table property could have different properties for wal_retention_secs_ and
// retain_delete_markers_ but still be equivalent.
bool EquivalentForDataCopy(const Schema& other) const {
if (this == &other) return true;
if (this->num_key_columns_ != other.num_key_columns_) return false;
Expand Down
3 changes: 2 additions & 1 deletion src/yb/master/backfill_index.cc
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,8 @@ Status BackfillTable::AllowCompactionsToGCDeleteMarkers(
VLOG(2) << __func__ << ": Trying to lock index table for Write";
auto l = index_table_info->LockForWrite();
VLOG(2) << __func__ << ": locked index table for Write";
l->mutable_data()->pb.mutable_schema()->mutable_table_properties()->set_is_backfilling(false);
l->mutable_data()->pb.mutable_schema()->mutable_table_properties()
->set_retain_delete_markers(false);

// Update sys-catalog with the new indexed table info.
TRACE("Updating index table metadata on disk");
Expand Down
8 changes: 4 additions & 4 deletions src/yb/master/catalog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2341,10 +2341,10 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
if ((req.has_index_info() || req.has_indexed_table_id()) &&
index_backfill_enabled &&
!req.skip_index_backfill()) {
// Start off the index table with major compactions disabled. We need this to preserve
// the delete markers until the backfill process is completed.
// No need to set index_permissions in the index table.
schema.SetIsBackfilling(true);
// Start off the index table with major compactions disabled. We need this to retain the delete
// markers until the backfill process is completed. No need to set index_permissions in the
// index table.
schema.SetRetainDeleteMarkers(true);
}

LOG(INFO) << "CreateTable with IndexInfo " << yb::ToString(index_info);
Expand Down
2 changes: 1 addition & 1 deletion src/yb/tablet/tablet.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2006,7 +2006,7 @@ Status Tablet::MarkBackfillDone() {
<< table_info->schema.ToString();
const vector<DeletedColumn> empty_deleted_cols;
Schema new_schema = Schema(table_info->schema);
new_schema.SetIsBackfilling(false);
new_schema.SetRetainDeleteMarkers(false);
metadata_->SetSchema(
new_schema, table_info->index_map, empty_deleted_cols, table_info->schema_version);
return metadata_->Flush();
Expand Down
2 changes: 1 addition & 1 deletion src/yb/tablet/tablet_retention_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void TabletRetentionPolicy::UnregisterReaderTimestamp(HybridTime timestamp) {
bool TabletRetentionPolicy::ShouldRetainDeleteMarkersInMajorCompaction() const {
// If the index table is in the process of being backfilled, then we
// want to retain delete markers until the backfill process is complete.
return metadata_.schema()->table_properties().IsBackfilling();
return metadata_.schema()->table_properties().retain_delete_markers();
}

HybridTime TabletRetentionPolicy::HistoryCutoffToPropagate(HybridTime last_write_ht) {
Expand Down

0 comments on commit 9f1733f

Please sign in to comment.