This is an automated email from the ASF dual-hosted git repository.
laiyingchun 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 80c8dde [tools] Fix schema version incorrctly set to 0 bug after
rebuilding master
80c8dde is described below
commit 80c8dde02310c94a65bed5acdfc28e332e334569
Author: Yingchun Lai <[email protected]>
AuthorDate: Tue Dec 21 18:15:19 2021 +0800
[tools] Fix schema version incorrctly set to 0 bug after rebuilding master
If we rebuild master for tables with 0 schema version, we may get
errors like following on master:
TS xxx (xxx:xxx) has reported a schema version greater than the current one
for tablet xxx (table xxx [id=xxx]). Expected version 0 got n (corruption)
...
TS xxx (xxx:xxx): alter failed for tablet xxx (table xxx [id=xxx]),no
further retry: Invalid argument: Tablet has a newer schema
or
TS xxx (xxx:xxx): alter failed for tablet xxx (table xxx [id=xxx]),no
further retry: Corruption: got a different schema for the same version number
Change-Id: Icf8b7c0c45bcc6160f6eabf977968de5a88ef5c7
Reviewed-on: http://gerrit.cloudera.org:8080/18112
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <[email protected]>
---
src/kudu/tools/kudu-admin-test.cc | 61 ++++++++++++++++++++++++++++++++++++
src/kudu/tools/kudu-tool-test.cc | 6 ++--
src/kudu/tools/master_rebuilder.cc | 40 ++++++++++++++---------
src/kudu/tools/tool_action_common.cc | 1 +
src/kudu/tools/tool_action_master.cc | 3 +-
src/kudu/tserver/tablet_service.cc | 1 +
src/kudu/tserver/tserver.proto | 1 +
7 files changed, 95 insertions(+), 18 deletions(-)
diff --git a/src/kudu/tools/kudu-admin-test.cc
b/src/kudu/tools/kudu-admin-test.cc
index cd6c9dd..5f3a5bb 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -3033,6 +3033,67 @@ TEST_F(AdminCliTest, TestRebuildMasterWithTombstones) {
NO_FATALS(cv.CheckCluster());
}
+TEST_F(AdminCliTest, TestRebuildMasterAndAddColumns) {
+ FLAGS_num_tablet_servers = 1;
+ FLAGS_num_replicas = 1;
+
+ NO_FATALS(BuildAndStart());
+
+ {
+ unique_ptr<KuduTableAlterer>
table_alterer(client_->NewTableAlterer(kTableId));
+ table_alterer->AddColumn("old_column_0")->Type(KuduColumnSchema::INT32);
+ ASSERT_OK(table_alterer->Alter());
+ }
+
+ // Shut down the master and wipe out its data.
+ NO_FATALS(cluster_->master()->Shutdown());
+ ASSERT_OK(cluster_->master()->DeleteFromDisk());
+
+ // Rebuild the master with the tool.
+ string stdout;
+ ASSERT_OK(RunKuduTool(RebuildMasterCmd(*cluster_, /*is_secure*/false,
/*log_to_stderr*/true),
+ &stdout));
+ ASSERT_STR_CONTAINS(stdout, "Rebuilt from 1 tablet servers, of which 0 had
errors");
+ ASSERT_STR_CONTAINS(stdout, "Rebuilt from 1 replicas, of which 0 had
errors");
+
+ // Restart the master and the tablet servers.
+ // The tablet servers must be restarted so they accept the new master's
certs.
+ for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+ cluster_->tablet_server(i)->Shutdown();
+ }
+ ASSERT_OK(cluster_->Restart());
+
+ ASSERT_OK(cluster_->WaitForTabletsRunning(cluster_->tablet_server(0),
+ 1,
+ MonoDelta::FromSeconds(10)));
+
+ // The client has to be rebuilt since there's a new master.
+ KuduClientBuilder builder;
+ ASSERT_OK(cluster_->CreateClient(&builder, &client_));
+
+ // Make sure we can add columns to the table we rebuilt.
+ {
+ unique_ptr<KuduTableAlterer>
table_alterer(client_->NewTableAlterer(kTableId));
+ table_alterer->AddColumn("new_column_0")->Type(KuduColumnSchema::INT32);
+ ASSERT_OK(table_alterer->Alter());
+ }
+
+ const string& ts_addr =
cluster_->tablet_server(0)->bound_rpc_addr().ToString();
+ ASSERT_EVENTUALLY([&]() {
+ string stdout;
+ ASSERT_OK(RunKuduTool({"remote_replica", "list", ts_addr,
"-include_schema"} , &stdout));
+ ASSERT_STR_CONTAINS(stdout, "new_column_0");
+ });
+
+ // Check the altered table is readable.
+ {
+ vector<string> rows;
+ client::sp::shared_ptr<KuduTable> table;
+ ASSERT_OK(client_->OpenTable(kTableId, &table));
+ ScanTableToStrings(table.get(), &rows);
+ }
+}
+
class SecureClusterAdminCliTest : public KuduTest {
public:
void SetUpCluster(bool is_secure) {
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 67c0193..cb95973 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -950,9 +950,9 @@ TEST_F(ToolTest, TestHelpXML) {
string stderr;
Status s = RunTool("--helpxml", &stdout, &stderr, nullptr, nullptr);
- ASSERT_TRUE(s.IsRuntimeError());
- ASSERT_FALSE(stdout.empty());
- ASSERT_TRUE(stderr.empty());
+ ASSERT_TRUE(s.IsRuntimeError()) << s.ToString();
+ ASSERT_FALSE(stdout.empty()) << stdout;
+ ASSERT_TRUE(stderr.empty()) << stderr;
// All wrapped in AllModes node
ASSERT_STR_MATCHES(stdout, "<\\?xml
version=\"1.0\"\\?><AllModes>.*</AllModes>");
diff --git a/src/kudu/tools/master_rebuilder.cc
b/src/kudu/tools/master_rebuilder.cc
index 27c9e3c..d6267b5 100644
--- a/src/kudu/tools/master_rebuilder.cc
+++ b/src/kudu/tools/master_rebuilder.cc
@@ -25,7 +25,7 @@
#include <string>
#include <vector>
-#include <gflags/gflags_declare.h>
+#include <gflags/gflags.h>
#include <glog/logging.h>
#include "kudu/common/common.pb.h"
@@ -56,6 +56,12 @@
#include "kudu/util/status.h"
DECLARE_int32(default_num_replicas);
+DEFINE_uint32(default_schema_version, 0, "The table schema version assigned to
tables if one "
+ "cannot be determined automatically. When the tablet server
version is < 1.16, a "
+ "viable value can be determined manually using "
+ "'kudu pbc dump tablet-meta/<tablet-id>' on each server and
taking the max value "
+ "found across all tablets. In tablet server versions >= 1.16,
Kudu will determine "
+ "the proper value for each tablet automatically.");
using kudu::master::Master;
using kudu::master::MasterOptions;
@@ -178,24 +184,24 @@ Status MasterRebuilder::CheckTableAndTabletConsistency(
void MasterRebuilder::CreateTable(const
ListTabletsResponsePB::StatusAndSchemaPB& replica) {
scoped_refptr<TableInfo> table(new TableInfo(oid_generator_.Next()));
table->mutable_metadata()->StartMutation();
- SysTablesEntryPB* table_metadata =
&table->mutable_metadata()->mutable_dirty()->pb;
+ SysTablesEntryPB* metadata = &table->mutable_metadata()->mutable_dirty()->pb;
const string& table_name = replica.tablet_status().table_name();
- table_metadata->set_name(table_name);
+ metadata->set_name(table_name);
- // We can't tell the schema version from ListTablets.
- // If there's multiple schemas reported by the replicas (i.e. if an alter
- // table is in progress), we'll fail to recover some replicas and only
- // partially recover the table.
- table_metadata->set_version(0);
+ if (!replica.has_schema_version()) {
+ metadata->set_version(FLAGS_default_schema_version);
+ } else {
+ metadata->set_version(replica.schema_version());
+ }
// We can't tell the replication factor from ListTablets.
// We'll guess the default replication factor because it's safe and almost
// always correct.
// TODO(awong): there's probably a better heuristic based on the number of
// replicas reported by the tablet servers.
- table_metadata->set_num_replicas(FLAGS_default_num_replicas);
- table_metadata->mutable_schema()->CopyFrom(replica.schema());
-
table_metadata->mutable_partition_schema()->CopyFrom(replica.partition_schema());
+ metadata->set_num_replicas(FLAGS_default_num_replicas);
+ metadata->mutable_schema()->CopyFrom(replica.schema());
+ metadata->mutable_partition_schema()->CopyFrom(replica.partition_schema());
int32_t max_column_id = 0;
for (const auto& column : replica.schema().columns()) {
@@ -203,9 +209,9 @@ void MasterRebuilder::CreateTable(const
ListTabletsResponsePB::StatusAndSchemaPB
max_column_id = column.id();
}
}
- table_metadata->set_next_column_id(max_column_id + 1);
- table_metadata->set_state(SysTablesEntryPB::RUNNING);
- table_metadata->set_state_msg("reconstructed by MasterRebuilder");
+ metadata->set_next_column_id(max_column_id + 1);
+ metadata->set_state(SysTablesEntryPB::RUNNING);
+ metadata->set_state_msg("reconstructed by MasterRebuilder");
table->mutable_metadata()->CommitMutation();
InsertOrDie(&tables_by_name_, table_name, table);
}
@@ -226,6 +232,12 @@ Status MasterRebuilder::CheckTableConsistency(
metadata->set_next_column_id(schema_from_replica.max_col_id() + 1);
}
+ // Update schema_version if needed.
+ if (replica.has_schema_version() &&
+ replica.schema_version() > metadata->version()) {
+ metadata->set_version(replica.schema_version());
+ }
+
// Check the schemas match.
Schema schema_from_table;
RETURN_NOT_OK(SchemaFromPB(metadata->schema(), &schema_from_table));
diff --git a/src/kudu/tools/tool_action_common.cc
b/src/kudu/tools/tool_action_common.cc
index 482357b..a8cfb03 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -508,6 +508,7 @@ Status GetServerStatus(const string& address, uint16_t
default_port,
Status GetReplicas(TabletServerServiceProxy* proxy,
vector<ListTabletsResponsePB::StatusAndSchemaPB>* replicas)
{
ListTabletsRequestPB req;
+ req.set_need_schema_info(true);
ListTabletsResponsePB resp;
RpcController rpc;
rpc.set_timeout(MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
diff --git a/src/kudu/tools/tool_action_master.cc
b/src/kudu/tools/tool_action_master.cc
index 919b1c6..4764be1 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -938,10 +938,11 @@ unique_ptr<Mode> BuildMasterMode() {
.Description("Rebuild a Kudu master from tablet server metadata")
.ExtraDescription(rebuild_extra_description)
.AddRequiredVariadicParameter({ kTabletServerAddressArg,
kTabletServerAddressDesc })
+ .AddOptionalParameter("default_num_replicas")
+ .AddOptionalParameter("default_schema_version")
.AddOptionalParameter("fs_data_dirs")
.AddOptionalParameter("fs_metadata_dir")
.AddOptionalParameter("fs_wal_dir")
- .AddOptionalParameter("default_num_replicas")
.Build();
builder.AddAction(std::move(unsafe_rebuild));
}
diff --git a/src/kudu/tserver/tablet_service.cc
b/src/kudu/tserver/tablet_service.cc
index f7a5c34..d031ebe 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -2250,6 +2250,7 @@ void TabletServiceImpl::ListTablets(const
ListTabletsRequestPB* req,
status->mutable_schema()));
CHECK_OK(replica->tablet_metadata()->partition_schema().ToPB(
replica->tablet_metadata()->schema(),
status->mutable_partition_schema()));
+ status->set_schema_version(replica->tablet_metadata()->schema_version());
}
}
context->RespondSuccess();
diff --git a/src/kudu/tserver/tserver.proto b/src/kudu/tserver/tserver.proto
index a5d0a9e..d464988 100644
--- a/src/kudu/tserver/tserver.proto
+++ b/src/kudu/tserver/tserver.proto
@@ -205,6 +205,7 @@ message ListTabletsResponsePB {
// set 'need_schema_info'.
optional SchemaPB schema = 2;
optional PartitionSchemaPB partition_schema = 3;
+ optional uint32 schema_version = 4;
}
repeated StatusAndSchemaPB status_and_schema = 2;
