From a013ba33e4ce131353edc71e27053b1801ffb8f7 Mon Sep 17 00:00:00 2001 From: dl239 Date: Thu, 31 Aug 2023 19:42:02 +0800 Subject: [PATCH] refactor: optimize the error message of create table (#3434) --- src/catalog/distribute_iterator_test.cc | 42 +++++++-------- src/client/tablet_client.cc | 68 +++---------------------- src/client/tablet_client.h | 7 +-- src/nameserver/name_server_impl.cc | 60 ++++++++++++---------- src/nameserver/name_server_impl.h | 4 +- src/replica/binlog_test.cc | 12 ++--- src/replica/snapshot_replica_test.cc | 56 ++++++++++---------- src/schema/index_util.cc | 14 ++--- src/schema/index_util.h | 2 +- src/sdk/node_adapter.cc | 34 +++++++------ src/test/util.cc | 35 +++++++++++++ src/test/util.h | 8 +++ 12 files changed, 168 insertions(+), 174 deletions(-) diff --git a/src/catalog/distribute_iterator_test.cc b/src/catalog/distribute_iterator_test.cc index 72a716bac3d..49e4958101f 100644 --- a/src/catalog/distribute_iterator_test.cc +++ b/src/catalog/distribute_iterator_test.cc @@ -162,8 +162,8 @@ TEST_F(DistributeIteratorTest, AllInRemote) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 4)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {4, client2}}; FullTableIterator it(tid, {}, tablet_clients); it.SeekToFirst(); @@ -205,8 +205,8 @@ TEST_F(DistributeIteratorTest, Hybrid) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 4)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {4, client2}}; FullTableIterator it(tid, tables, tablet_clients); it.SeekToFirst(); @@ -265,8 +265,8 @@ TEST_F(DistributeIteratorTest, FullTableTraverseLimit) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 4)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {4, client2}}; FullTableIterator it(tid, tables, tablet_clients); it.SeekToFirst(); @@ -317,7 +317,7 @@ TEST_F(DistributeIteratorTest, TraverseLimitSingle) { auto client1 = std::make_shared(endpoints[0], endpoints[0]); ASSERT_EQ(client1->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 0)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); std::map> tablet_clients = {{0, client1}}; for (int i = 0; i < 10; i++) { std::string key = "card" + std::to_string(i); @@ -355,8 +355,8 @@ TEST_F(DistributeIteratorTest, TraverseLimit) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; std::map cout_map = {{0, 0}, {1, 0}, {2, 0}, {3, 0}}; for (int i = 0; i < 100; i++) { @@ -399,8 +399,8 @@ TEST_F(DistributeIteratorTest, WindowIterator) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; for (int i = 0; i < 20; i++) { std::string key = "card" + std::to_string(i); @@ -462,7 +462,7 @@ TEST_F(DistributeIteratorTest, RemoteIterator) { auto client1 = std::make_shared(endpoints[0], endpoints[0]); ASSERT_EQ(client1->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 0)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); std::map> tablet_clients = {{0, client1}}; codec::SDKCodec codec(metas[0]); int64_t now = 1999; @@ -540,7 +540,7 @@ TEST_F(DistributeIteratorTest, RemoteIteratorSecondIndex) { auto client1 = std::make_shared(endpoints[0], endpoints[0]); ASSERT_EQ(client1->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 0)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); std::map> tablet_clients = {{0, client1}}; codec::SDKCodec codec(metas[0]); uint64_t now = ::baidu::common::timer::get_micros() / 1000; @@ -678,8 +678,8 @@ TEST_F(DistributeIteratorTest, MoreTsCnt) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; std::map cout_map = {{0, 0}, {1, 0}, {2, 0}, {3, 0}}; for (int i = 0; i < 50; i++) { @@ -759,8 +759,8 @@ TEST_F(DistributeIteratorTest, TraverseSameTs) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; std::map cout_map = {{0, 0}, {1, 0}, {2, 0}, {3, 0}}; for (int i = 0; i < 20; i++) { @@ -804,8 +804,8 @@ TEST_F(DistributeIteratorTest, WindowIteratorLimit) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; for (int i = 0; i < 20; i++) { std::string key = "card" + std::to_string(i); @@ -921,8 +921,8 @@ TEST_F(DistributeIteratorTest, IteratorZero) { auto client2 = std::make_shared(endpoints[1], endpoints[1]); ASSERT_EQ(client2->Init(), 0); std::vector<::openmldb::api::TableMeta> metas = {CreateTableMeta(tid, 1), CreateTableMeta(tid, 3)}; - ASSERT_TRUE(client1->CreateTable(metas[0])); - ASSERT_TRUE(client2->CreateTable(metas[1])); + ASSERT_TRUE(client1->CreateTable(metas[0]).OK()); + ASSERT_TRUE(client2->CreateTable(metas[1]).OK()); std::map> tablet_clients = {{1, client1}, {3, client2}}; std::map cout_map = {{0, 0}, {1, 0}, {2, 0}, {3, 0}}; int expect = 0; diff --git a/src/client/tablet_client.cc b/src/client/tablet_client.cc index aca61c6cad9..3ed9eb596b5 100644 --- a/src/client/tablet_client.cc +++ b/src/client/tablet_client.cc @@ -153,72 +153,18 @@ bool TabletClient::SQLBatchRequestQuery(const std::string& db, const std::string return true; } -bool TabletClient::CreateTable(const std::string& name, uint32_t tid, uint32_t pid, uint64_t abs_ttl, uint64_t lat_ttl, - bool leader, const std::vector& endpoints, - const ::openmldb::type::TTLType& type, uint32_t seg_cnt, uint64_t term, - const ::openmldb::type::CompressType compress_type, - ::openmldb::common::StorageMode storage_mode) { - ::openmldb::api::CreateTableRequest request; - if (type == ::openmldb::type::kLatestTime) { - if (lat_ttl > FLAGS_latest_ttl_max) { - return false; - } - } else if (type == ::openmldb::type::TTLType::kAbsoluteTime) { - if (abs_ttl > FLAGS_absolute_ttl_max) { - return false; - } - } else { - if (abs_ttl > FLAGS_absolute_ttl_max || lat_ttl > FLAGS_latest_ttl_max) { - return false; - } - } - ::openmldb::api::TableMeta* table_meta = request.mutable_table_meta(); - table_meta->set_name(name); - table_meta->set_tid(tid); - table_meta->set_pid(pid); - table_meta->set_compress_type(compress_type); - table_meta->set_seg_cnt(seg_cnt); - table_meta->set_storage_mode(storage_mode); - if (leader) { - table_meta->set_mode(::openmldb::api::TableMode::kTableLeader); - table_meta->set_term(term); - } else { - table_meta->set_mode(::openmldb::api::TableMode::kTableFollower); - } - for (size_t i = 0; i < endpoints.size(); i++) { - table_meta->add_replicas(endpoints[i]); - } - ::openmldb::common::ColumnDesc* column_desc = table_meta->add_column_desc(); - column_desc->set_name("idx0"); - column_desc->set_data_type(::openmldb::type::kString); - ::openmldb::common::ColumnKey* index = table_meta->add_column_key(); - index->set_index_name("idx0"); - index->add_col_name("idx0"); - ::openmldb::common::TTLSt* ttl = index->mutable_ttl(); - ttl->set_abs_ttl(abs_ttl); - ttl->set_lat_ttl(lat_ttl); - ttl->set_ttl_type(type); - // table_meta->set_ttl_type(type); - ::openmldb::api::CreateTableResponse response; - bool ok = client_.SendRequest(&::openmldb::api::TabletServer_Stub::CreateTable, &request, &response, - FLAGS_request_timeout_ms * 2, 1); - if (ok && response.code() == 0) { - return true; - } - return false; -} - -bool TabletClient::CreateTable(const ::openmldb::api::TableMeta& table_meta) { +base::Status TabletClient::CreateTable(const ::openmldb::api::TableMeta& table_meta) { ::openmldb::api::CreateTableRequest request; ::openmldb::api::TableMeta* table_meta_ptr = request.mutable_table_meta(); table_meta_ptr->CopyFrom(table_meta); ::openmldb::api::CreateTableResponse response; - bool ok = client_.SendRequest(&::openmldb::api::TabletServer_Stub::CreateTable, &request, &response, - FLAGS_request_timeout_ms * 2, 1); - if (ok && response.code() == 0) { - return true; + if (!client_.SendRequest(&::openmldb::api::TabletServer_Stub::CreateTable, &request, &response, + FLAGS_request_timeout_ms * 2, 1)) { + return {base::ReturnCode::kRPCError, "send request failed!"}; + } else if (response.code() == 0) { + return {}; } - return false; + return {response.code(), response.msg()}; } bool TabletClient::UpdateTableMetaForAddField(uint32_t tid, const std::vector& cols, diff --git a/src/client/tablet_client.h b/src/client/tablet_client.h index 895960b9bdc..23a1c5df879 100644 --- a/src/client/tablet_client.h +++ b/src/client/tablet_client.h @@ -54,12 +54,7 @@ class TabletClient : public Client { int Init() override; - bool CreateTable(const std::string& name, uint32_t tid, uint32_t pid, uint64_t abs_ttl, uint64_t lat_ttl, - bool leader, const std::vector& endpoints, const ::openmldb::type::TTLType& type, - uint32_t seg_cnt, uint64_t term, const ::openmldb::type::CompressType compress_type, - ::openmldb::common::StorageMode storage_mode = ::openmldb::common::kMemory); - - bool CreateTable(const ::openmldb::api::TableMeta& table_meta); + base::Status CreateTable(const ::openmldb::api::TableMeta& table_meta); bool UpdateTableMetaForAddField(uint32_t tid, const std::vector& cols, const openmldb::common::VersionPair& pair, diff --git a/src/nameserver/name_server_impl.cc b/src/nameserver/name_server_impl.cc index 1dd5acf58ff..4aae3c594c3 100644 --- a/src/nameserver/name_server_impl.cc +++ b/src/nameserver/name_server_impl.cc @@ -2182,9 +2182,9 @@ int NameServerImpl::SetPartitionInfo(TableInfo& table_info) { return 0; } -int NameServerImpl::CreateTableOnTablet(const std::shared_ptr<::openmldb::nameserver::TableInfo>& table_info, - bool is_leader, std::map>& endpoint_map, - uint64_t term) { +base::Status NameServerImpl::CreateTableOnTablet(const std::shared_ptr<::openmldb::nameserver::TableInfo>& table_info, + bool is_leader, uint64_t term, + std::map>* endpoint_map) { ::openmldb::type::CompressType compress_type = ::openmldb::type::CompressType::kNoCompress; if (table_info->compress_type() == ::openmldb::type::kSnappy) { compress_type = ::openmldb::type::CompressType::kSnappy; @@ -2201,18 +2201,16 @@ int NameServerImpl::CreateTableOnTablet(const std::shared_ptr<::openmldb::namese table_meta.set_key_entry_max_height(table_info->key_entry_max_height()); } for (int idx = 0; idx < table_info->column_desc_size(); idx++) { - ::openmldb::common::ColumnDesc* column_desc = table_meta.add_column_desc(); - column_desc->CopyFrom(table_info->column_desc(idx)); + table_meta.add_column_desc()->CopyFrom(table_info->column_desc(idx)); } for (int idx = 0; idx < table_info->column_key_size(); idx++) { - ::openmldb::common::ColumnKey* column_key = table_meta.add_column_key(); - column_key->CopyFrom(table_info->column_key(idx)); + table_meta.add_column_key()->CopyFrom(table_info->column_key(idx)); } for (const auto& table_partition : table_info->table_partition()) { - ::openmldb::common::TablePartition* partition = table_meta.add_table_partition(); + auto partition = table_meta.add_table_partition(); partition->set_pid(table_partition.pid()); for (const auto& partition_meta : table_partition.partition_meta()) { - ::openmldb::common::PartitionMeta* meta = partition->add_partition_meta(); + auto meta = partition->add_partition_meta(); meta->set_endpoint(partition_meta.endpoint()); meta->set_is_leader(partition_meta.is_leader()); meta->set_is_alive(true); @@ -2229,36 +2227,37 @@ int NameServerImpl::CreateTableOnTablet(const std::shared_ptr<::openmldb::namese std::string endpoint = table_info->table_partition(idx).partition_meta(meta_idx).endpoint(); auto tablet_ptr = GetTablet(endpoint); if (!tablet_ptr) { - PDLOG(WARNING, "endpoint[%s] can not find client", endpoint.c_str()); - return -1; + PDLOG(WARNING, "endpoint[%s] cannot find client", endpoint.c_str()); + return {base::ReturnCode::kServerConnError, absl::StrCat("endpoint ", endpoint, " cannot find client")}; } if (is_leader) { - ::openmldb::nameserver::TablePartition* table_partition = table_info->mutable_table_partition(idx); - ::openmldb::nameserver::TermPair* term_pair = table_partition->add_term_offset(); + auto table_partition = table_info->mutable_table_partition(idx); + auto term_pair = table_partition->add_term_offset(); term_pair->set_term(term); term_pair->set_offset(0); table_meta.set_mode(::openmldb::api::TableMode::kTableLeader); table_meta.set_term(term); - for (const auto& e : endpoint_map[pid]) { + for (const auto& e : (*endpoint_map)[pid]) { table_meta.add_replicas(e); } } else { - if (endpoint_map.find(pid) == endpoint_map.end()) { - endpoint_map.insert(std::make_pair(pid, std::vector())); + auto iter = endpoint_map->find(pid); + if (iter == endpoint_map->end()) { + iter = endpoint_map->emplace(pid, std::vector()).first; } - endpoint_map[pid].push_back(endpoint); + iter->second.push_back(endpoint); table_meta.set_mode(::openmldb::api::TableMode::kTableFollower); } - if (!tablet_ptr->client_->CreateTable(table_meta)) { - PDLOG(WARNING, "create table failed. tid[%u] pid[%u] endpoint[%s]", table_info->tid(), pid, - endpoint.c_str()); - return -1; + if (auto status = tablet_ptr->client_->CreateTable(table_meta); !status.OK()) { + PDLOG(WARNING, "create table failed. tid[%u] pid[%u] endpoint[%s] msg[%s]", + table_info->tid(), pid, endpoint.c_str(), status.GetMsg().c_str()); + return status; } PDLOG(INFO, "create table success. tid[%u] pid[%u] endpoint[%s] idx[%d]", table_info->tid(), pid, endpoint.c_str(), idx); } } - return 0; + return {}; } int NameServerImpl::DropTableOnTablet(std::shared_ptr<::openmldb::nameserver::TableInfo> table_info) { @@ -3758,11 +3757,18 @@ void NameServerImpl::CreateTableInternel(GeneralResponse& response, std::shared_ptr<::openmldb::api::TaskInfo> task_ptr) { std::map> endpoint_map; do { - if (CreateTableOnTablet(table_info, false, endpoint_map, cur_term) < 0 || - CreateTableOnTablet(table_info, true, endpoint_map, cur_term) < 0) { - response.set_code(::openmldb::base::ReturnCode::kCreateTableFailedOnTablet); - response.set_msg("create table failed on tablet"); - PDLOG(WARNING, "create table failed. name[%s] tid[%u]", table_info->name().c_str(), tid); + auto status = CreateTableOnTablet(table_info, false, cur_term, &endpoint_map); + if (!status.OK()) { + base::SetResponseStatus(status, &response); + PDLOG(WARNING, "create table failed. name[%s] tid[%u] msg[%s]", + table_info->name().c_str(), tid, status.GetMsg().c_str()); + break; + } + status = CreateTableOnTablet(table_info, true, cur_term, &endpoint_map); + if (!status.OK()) { + base::SetResponseStatus(status, &response); + PDLOG(WARNING, "create table failed. name[%s] tid[%u] msg[%s]", + table_info->name().c_str(), tid, status.GetMsg().c_str()); break; } if (!IsClusterMode()) { diff --git a/src/nameserver/name_server_impl.h b/src/nameserver/name_server_impl.h index d7e030c0eab..4bfe84ad5f4 100644 --- a/src/nameserver/name_server_impl.h +++ b/src/nameserver/name_server_impl.h @@ -306,8 +306,8 @@ class NameServerImpl : public NameServer { const ::openmldb::nameserver::TableInfo& table_info_local, uint32_t pid, int& code, // NOLINT std::string& msg); // NOLINT - int CreateTableOnTablet(const std::shared_ptr<::openmldb::nameserver::TableInfo>& table_info, bool is_leader, - std::map>& endpoint_map, uint64_t term); // NOLINT + base::Status CreateTableOnTablet(const std::shared_ptr<::openmldb::nameserver::TableInfo>& table_info, + bool is_leader, uint64_t term, std::map>* endpoint_map); void CheckZkClient(); diff --git a/src/replica/binlog_test.cc b/src/replica/binlog_test.cc index 350a53d4d04..797514c76bd 100644 --- a/src/replica/binlog_test.cc +++ b/src/replica/binlog_test.cc @@ -86,19 +86,19 @@ TEST_F(BinlogTest, DeleteBinlog) { ::openmldb::client::TabletClient client(leader_point, ""); client.Init(); std::vector endpoints; - bool ret = - client.CreateTable("table1", tid, pid, 100000, 0, true, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, 16, - 0, ::openmldb::type::CompressType::kNoCompress); - ASSERT_TRUE(ret); + auto status = client.CreateTable(test::CreateTableMeta("table1", tid, pid, 100000, 0, true, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, 16, + 0, ::openmldb::type::CompressType::kNoCompress)); + ASSERT_TRUE(status.OK()); uint64_t cur_time = ::baidu::common::timer::get_micros() / 1000; int count = 1000; while (count) { std::string key = "testkey_" + std::to_string(count); - ret = client.Put(tid, pid, key, cur_time, ::openmldb::test::EncodeKV(key, std::string(10 * 1024, 'a'))); + client.Put(tid, pid, key, cur_time, ::openmldb::test::EncodeKV(key, std::string(10 * 1024, 'a'))); count--; } - ret = client.MakeSnapshot(tid, pid, 0); + auto ret = client.MakeSnapshot(tid, pid, 0); std::string binlog_path = FLAGS_db_root_path + "/2_123/binlog"; std::vector vec; ASSERT_TRUE(ret); diff --git a/src/replica/snapshot_replica_test.cc b/src/replica/snapshot_replica_test.cc index ee6c67e0b36..a9302050142 100644 --- a/src/replica/snapshot_replica_test.cc +++ b/src/replica/snapshot_replica_test.cc @@ -82,13 +82,13 @@ TEST_P(SnapshotReplicaTest, AddReplicate) { ::openmldb::client::TabletClient client(leader_point, ""); client.Init(); std::vector endpoints; - bool ret = - client.CreateTable("table1", tid, pid, 100000, 0, true, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, 16, - 0, ::openmldb::type::CompressType::kNoCompress, storage_mode); - ASSERT_TRUE(ret); + auto status = client.CreateTable(test::CreateTableMeta("table1", tid, pid, 100000, 0, true, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, 16, 0, + ::openmldb::type::CompressType::kNoCompress, storage_mode)); + ASSERT_TRUE(status.OK()); std::string end_point = "127.0.0.1:18530"; - ret = client.AddReplica(tid, pid, end_point); + auto ret = client.AddReplica(tid, pid, end_point); ASSERT_TRUE(ret); sleep(1); @@ -123,12 +123,12 @@ TEST_P(SnapshotReplicaTest, LeaderAndFollower) { ::openmldb::client::TabletClient client(leader_point, ""); client.Init(); std::vector endpoints; - bool ret = - client.CreateTable("table1", tid, pid, 100000, 0, true, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, 16, - 0, ::openmldb::type::CompressType::kNoCompress, storage_mode); - ASSERT_TRUE(ret); + auto status = client.CreateTable(test::CreateTableMeta("table1", tid, pid, 100000, 0, true, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, 16, + 0, ::openmldb::type::CompressType::kNoCompress, storage_mode)); + ASSERT_TRUE(status.OK()); uint64_t cur_time = ::baidu::common::timer::get_micros() / 1000; - ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); + auto ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); ASSERT_TRUE(ret); uint32_t count = 0; @@ -159,9 +159,10 @@ TEST_P(SnapshotReplicaTest, LeaderAndFollower) { // server.RunUntilAskedToQuit(); ::openmldb::client::TabletClient client1(follower_point, ""); client1.Init(); - ret = client1.CreateTable("table1", tid, pid, 14400, 0, false, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, - 16, 0, ::openmldb::type::CompressType::kNoCompress, storage_mode); - ASSERT_TRUE(ret); + status = client1.CreateTable(test::CreateTableMeta("table1", tid, pid, 14400, 0, false, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, + 16, 0, ::openmldb::type::CompressType::kNoCompress, storage_mode)); + ASSERT_TRUE(status.OK()); client.AddReplica(tid, pid, follower_point); sleep(3); @@ -232,12 +233,12 @@ TEST_P(SnapshotReplicaTest, SendSnapshot) { ::openmldb::client::TabletClient client(leader_point, ""); client.Init(); std::vector endpoints; - bool ret = - client.CreateTable("table1", tid, pid, 100000, 0, true, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, 16, - 0, ::openmldb::type::CompressType::kNoCompress, storage_mode); - ASSERT_TRUE(ret); + auto status = client.CreateTable(test::CreateTableMeta("table1", tid, pid, 100000, 0, true, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, 16, + 0, ::openmldb::type::CompressType::kNoCompress, storage_mode)); + ASSERT_TRUE(status.OK()); uint64_t cur_time = ::baidu::common::timer::get_micros() / 1000; - ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); + auto ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); ASSERT_TRUE(ret); uint32_t count = 0; @@ -344,11 +345,11 @@ TEST_P(SnapshotReplicaTest, IncompleteSnapshot) { ::openmldb::client::TabletClient client(leader_point, ""); client.Init(); std::vector endpoints; - bool ret = - client.CreateTable("table1", tid, pid, 100000, 0, true, endpoints, ::openmldb::type::TTLType::kAbsoluteTime, - 16, 0, ::openmldb::type::CompressType::kNoCompress, storage_mode); - ASSERT_TRUE(ret); - ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); + auto status = client.CreateTable(test::CreateTableMeta("table1", tid, pid, 100000, 0, true, endpoints, + ::openmldb::type::TTLType::kAbsoluteTime, + 16, 0, ::openmldb::type::CompressType::kNoCompress, storage_mode)); + ASSERT_TRUE(status.OK()); + auto ret = client.Put(tid, pid, "testkey", cur_time, ::openmldb::test::EncodeKV("testkey", "value1")); ASSERT_TRUE(ret); uint32_t count = 0; @@ -571,8 +572,7 @@ TEST_P(SnapshotReplicaTest, LeaderAndFollowerTS) { SchemaCodec::SetIndex(table_meta.add_column_key(), "card1", "card", "ts2", ::openmldb::type::kAbsoluteTime, 0, 0); SchemaCodec::SetIndex(table_meta.add_column_key(), "mcc", "mcc", "ts2", ::openmldb::type::kAbsoluteTime, 0, 0); table_meta.set_mode(::openmldb::api::TableMode::kTableLeader); - bool ret = client.CreateTable(table_meta); - ASSERT_TRUE(ret); + ASSERT_TRUE(client.CreateTable(table_meta).OK()); uint64_t cur_time = ::baidu::common::timer::get_micros() / 1000; std::vector> dimensions; dimensions.push_back(std::make_pair("card0", 0)); @@ -582,8 +582,7 @@ TEST_P(SnapshotReplicaTest, LeaderAndFollowerTS) { std::vector row = {"card0", "mcc0", "1.3", std::to_string(cur_time), std::to_string(cur_time - 100)}; std::string value; sdk_codec.EncodeRow(row, &value); - ret = client.Put(tid, pid, cur_time, value, dimensions); - ASSERT_TRUE(ret); + ASSERT_TRUE(client.Put(tid, pid, cur_time, value, dimensions)); sleep(3); ::openmldb::test::TempPath temp_path; @@ -605,8 +604,7 @@ TEST_P(SnapshotReplicaTest, LeaderAndFollowerTS) { ::openmldb::client::TabletClient client1(follower_point, ""); client1.Init(); table_meta.set_mode(::openmldb::api::TableMode::kTableFollower); - ret = client1.CreateTable(table_meta); - ASSERT_TRUE(ret); + ASSERT_TRUE(client1.CreateTable(table_meta).OK()); client.AddReplica(tid, pid, follower_point); sleep(3); diff --git a/src/schema/index_util.cc b/src/schema/index_util.cc index 81ed5cd5177..fead40b5c70 100644 --- a/src/schema/index_util.cc +++ b/src/schema/index_util.cc @@ -80,19 +80,21 @@ base::Status IndexUtil::CheckIndex(const std::map FLAGS_absolute_ttl_max || ttl.lat_ttl() > FLAGS_latest_ttl_max) { - return false; +base::Status IndexUtil::CheckTTL(const ::openmldb::common::TTLSt& ttl) { + if (ttl.abs_ttl() > FLAGS_absolute_ttl_max) { + return {base::ReturnCode::kError, absl::StrCat("absolute ttl cannot be greater than ", FLAGS_absolute_ttl_max)}; + } else if (ttl.lat_ttl() > FLAGS_latest_ttl_max) { + return {base::ReturnCode::kError, absl::StrCat("latest ttl cannot be greater than ", FLAGS_latest_ttl_max)}; } - return true; + return {}; } bool IndexUtil::AddDefaultIndex(openmldb::nameserver::TableInfo* table_info) { diff --git a/src/schema/index_util.h b/src/schema/index_util.h index 465abac1ec1..31880fa0654 100644 --- a/src/schema/index_util.h +++ b/src/schema/index_util.h @@ -46,7 +46,7 @@ class IndexUtil { static base::Status CheckUnique(const PBIndex& index); - static bool CheckTTL(const ::openmldb::common::TTLSt& ttl); + static base::Status CheckTTL(const ::openmldb::common::TTLSt& ttl); static bool AddDefaultIndex(openmldb::nameserver::TableInfo* table_info); diff --git a/src/sdk/node_adapter.cc b/src/sdk/node_adapter.cc index bf64ac8d4bc..b148c8a4ca9 100644 --- a/src/sdk/node_adapter.cc +++ b/src/sdk/node_adapter.cc @@ -273,6 +273,10 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n *status = {hybridse::common::kUnsupportSql, "partitionnum should be great than 0"}; return false; } + if (storage_mode == hybridse::node::StorageMode::kUnknown) { + *status = {hybridse::common::kUnsupportSql, "invalid storage mode"}; + return false; + } // deny create table when invalid configuration in standalone mode if (!is_cluster_mode) { if (replica_num != 1) { @@ -298,7 +302,7 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n auto* column_def = dynamic_cast(column_desc); ::openmldb::common::ColumnDesc* add_column_desc = table->add_column_desc(); if (column_names.find(add_column_desc->name()) != column_names.end()) { - status->msg = "CREATE common: COLUMN NAME " + column_def->GetColumnName() + " duplicate"; + status->msg = "COLUMN NAME " + column_def->GetColumnName() + " duplicate"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -307,7 +311,7 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n column_names.insert(std::make_pair(column_def->GetColumnName(), add_column_desc)); openmldb::type::DataType data_type; if (!openmldb::schema::SchemaAdapter::ConvertType(column_def->GetColumnType(), &data_type)) { - status->msg = "CREATE common: column type " + + status->msg = "column type " + hybridse::node::DataTypeName(column_def->GetColumnType()) + " is not supported"; status->code = hybridse::common::kUnsupportSql; return false; @@ -316,14 +320,14 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n auto default_val = column_def->GetDefaultValue(); if (default_val) { if (default_val->GetExprType() != hybridse::node::kExprPrimary) { - status->msg = "CREATE common: default value expression not supported"; + status->msg = "default value expression not supported"; status->code = hybridse::common::kTypeError; return false; } auto val = TransformDataType(*dynamic_cast(default_val), add_column_desc->data_type()); if (!val) { - status->msg = "CREATE common: default value type mismatch"; + status->msg = "default value type mismatch"; status->code = hybridse::common::kTypeError; return false; } @@ -339,7 +343,7 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n DCHECK(index_name.empty()); index_name = PlanAPI::GenerateName("INDEX", table->column_key_size()); if (index_names.find(index_name) != index_names.end()) { - status->msg = "CREATE common: INDEX NAME " + index_name + " duplicate"; + status->msg = "INDEX NAME " + index_name + " duplicate"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -356,12 +360,12 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n } } if (!has_generate_index) { - status->msg = "CREATE common: can not found index col"; + status->msg = "can not found index col"; status->code = hybridse::common::kUnsupportSql; return false; } } else { - status->msg = "CREATE common: INDEX KEY empty"; + status->msg = "INDEX KEY empty"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -401,7 +405,7 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n auto p_meta_node = dynamic_cast(partition_meta); const std::string& ep = p_meta_node->GetEndpoint(); if (endpoint_set.count(ep) > 0) { - status->msg = "CREATE common: partition meta endpoint duplicate"; + status->msg = "partition meta endpoint duplicate"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -413,7 +417,7 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n } else if (p_meta_node->GetRoleType() == hybridse::node::kFollower) { meta->set_is_leader(false); } else { - status->msg = "CREATE common: role_type " + + status->msg = "role_type " + hybridse::node::RoleTypeName(p_meta_node->GetRoleType()) + " not support"; status->code = hybridse::common::kUnsupportSql; return false; @@ -475,7 +479,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i std::transform(ttl_type.begin(), ttl_type.end(), ttl_type.begin(), ::tolower); openmldb::type::TTLType type; if (!::openmldb::codec::SchemaCodec::TTLTypeParse(ttl_type, &type)) { - status->msg = "CREATE common: ttl_type " + column_index->ttl_type() + " not support"; + status->msg = "ttl_type " + column_index->ttl_type() + " not support"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -485,7 +489,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i } if (ttl_st->ttl_type() == openmldb::type::kAbsoluteTime) { if (column_index->GetAbsTTL() == -1 || column_index->GetLatTTL() != -2) { - status->msg = "CREATE common: abs ttl format error or set lat ttl"; + status->msg = "abs ttl format error or set lat ttl"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -498,7 +502,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i } } else if (ttl_st->ttl_type() == openmldb::type::kLatestTime) { if (column_index->GetLatTTL() == -1 || column_index->GetAbsTTL() != -2) { - status->msg = "CREATE common: lat ttl format error"; + status->msg = "lat ttl format error"; status->code = hybridse::common::kUnsupportSql; return false; } @@ -510,7 +514,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i } } else { if (column_index->GetAbsTTL() == -1) { - status->msg = "CREATE common: abs ttl format error for " + type::TTLType_Name(ttl_st->ttl_type()); + status->msg = "abs ttl format error for " + type::TTLType_Name(ttl_st->ttl_type()); status->code = hybridse::common::kUnsupportSql; return false; } @@ -520,7 +524,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i ttl_st->set_abs_ttl(base::AbsTTLConvert(column_index->GetAbsTTL(), true)); } if (column_index->GetLatTTL() == -1) { - status->msg = "CREATE common: lat ttl format error for " + type::TTLType_Name(ttl_st->ttl_type()); + status->msg = "lat ttl format error for " + type::TTLType_Name(ttl_st->ttl_type()); status->code = hybridse::common::kUnsupportSql; return false; } @@ -535,7 +539,7 @@ bool NodeAdapter::TransformToColumnKey(hybridse::node::ColumnIndexNode* column_i if (!column_names.empty()) { auto it = column_names.find(column_index->GetTs()); if (it == column_names.end()) { - status->msg = "CREATE common: TS NAME " + column_index->GetTs() + " not exists"; + status->msg = "TS NAME " + column_index->GetTs() + " not exists"; status->code = hybridse::common::kUnsupportSql; return false; } diff --git a/src/test/util.cc b/src/test/util.cc index 59a772eed78..ed07eb642d4 100644 --- a/src/test/util.cc +++ b/src/test/util.cc @@ -280,5 +280,40 @@ std::string TempPath::CreateTempPath(const std::string& prefix) { return path; } +api::TableMeta CreateTableMeta(const std::string& name, uint32_t tid, uint32_t pid, + uint64_t abs_ttl, uint64_t lat_ttl, + bool leader, const std::vector& endpoints, + const ::openmldb::type::TTLType& type, uint32_t seg_cnt, uint64_t term, + ::openmldb::type::CompressType compress_type, + ::openmldb::common::StorageMode storage_mode) { + api::TableMeta table_meta; + table_meta.set_name(name); + table_meta.set_tid(tid); + table_meta.set_pid(pid); + table_meta.set_compress_type(compress_type); + table_meta.set_seg_cnt(seg_cnt); + table_meta.set_storage_mode(storage_mode); + if (leader) { + table_meta.set_mode(::openmldb::api::TableMode::kTableLeader); + table_meta.set_term(term); + } else { + table_meta.set_mode(::openmldb::api::TableMode::kTableFollower); + } + for (size_t i = 0; i < endpoints.size(); i++) { + table_meta.add_replicas(endpoints[i]); + } + ::openmldb::common::ColumnDesc* column_desc = table_meta.add_column_desc(); + column_desc->set_name("idx0"); + column_desc->set_data_type(::openmldb::type::kString); + ::openmldb::common::ColumnKey* index = table_meta.add_column_key(); + index->set_index_name("idx0"); + index->add_col_name("idx0"); + ::openmldb::common::TTLSt* ttl = index->mutable_ttl(); + ttl->set_abs_ttl(abs_ttl); + ttl->set_lat_ttl(lat_ttl); + ttl->set_ttl_type(type); + return table_meta; +} + } // namespace test } // namespace openmldb diff --git a/src/test/util.h b/src/test/util.h index db466995dde..02bd3fd2a04 100644 --- a/src/test/util.h +++ b/src/test/util.h @@ -100,6 +100,14 @@ std::string GetExeDir(); std::string GetParentDir(const std::string& path); +api::TableMeta CreateTableMeta(const std::string& name, uint32_t tid, uint32_t pid, + uint64_t abs_ttl, uint64_t lat_ttl, + bool leader, const std::vector& endpoints, + const ::openmldb::type::TTLType& type, uint32_t seg_cnt, uint64_t term, + ::openmldb::type::CompressType compress_type, + common::StorageMode storage_mode = ::openmldb::common::kMemory); + + } // namespace test } // namespace openmldb #endif // SRC_TEST_UTIL_H_