Skip to content

Commit f2a2d0b

Browse files
spolitovspolitov
committed
[yugabyte#8543] PITR: Add test for need to increase table version on restore
Summary: This diff adds that fails if we don't patch schema version on restore. Consider the following scenario, w/o patching: 1) Create table. Schema version - 0. 2) Add text column to table. Schema version - 1. 3) Insert values into table. Each CQL proxy suppose schema version 1 for this table. 4) Restore to time between (1) and (2). Schema version - 0. 5) Add int column to table. Schema version - 1. 6) Try insert values with wrong type into table. So table has schema version 1, but new column is INT. CQL proxy suppose schema version is also 1, but the last column is TEXT. If we patch schema version on restore and set it to last version + 1, then each CQL proxy will have to reload schema for this table. Test Plan: ybd --gtest_filter YbAdminSnapshotScheduleTest.AlterTable Reviewers: bogdan Reviewed By: bogdan Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D11693
1 parent 588542a commit f2a2d0b

File tree

7 files changed

+214
-23
lines changed

7 files changed

+214
-23
lines changed

‎src/yb/integration-tests/cassandra_cpp_driver-test.cc‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ class CppCassandraDriverTest : public ExternalMiniClusterITestBase {
111111
hosts.push_back(cluster_->tablet_server(i)->bind_host());
112112
}
113113
driver_.reset(new CppCassandraDriver(
114-
hosts, cluster_->tablet_server(0)->cql_rpc_port(), UsePartitionAwareRouting()));
114+
hosts, cluster_->tablet_server(0)->cql_rpc_port(), use_partition_aware_routing()));
115115

116116
// Create and use default keyspace.
117117
auto deadline = CoarseMonoClock::now() + 15s;
@@ -153,8 +153,8 @@ class CppCassandraDriverTest : public ExternalMiniClusterITestBase {
153153
return 1;
154154
}
155155

156-
virtual bool UsePartitionAwareRouting() {
157-
return true;
156+
virtual UsePartitionAwareRouting use_partition_aware_routing() {
157+
return UsePartitionAwareRouting::kTrue;
158158
}
159159

160160
protected:
@@ -203,10 +203,10 @@ class CppCassandraDriverTestIndex : public CppCassandraDriverTest {
203203
};
204204
}
205205

206-
bool UsePartitionAwareRouting() override {
206+
UsePartitionAwareRouting use_partition_aware_routing() override {
207207
// Disable partition aware routing in this test because of TSAN issue (#1837).
208208
// Should be reenabled when issue is fixed.
209-
return false;
209+
return UsePartitionAwareRouting::kFalse;
210210
}
211211

212212
protected:
@@ -2077,7 +2077,7 @@ TEST_F_EX(CppCassandraDriverTest, TestDeleteAndCreateIndex, CppCassandraDriverTe
20772077
}
20782078
for (int i = 0; i <= kNumLoops; i++) {
20792079
drivers.emplace_back(new CppCassandraDriver(
2080-
hosts, cluster_->tablet_server(0)->cql_rpc_port(), false /*UsePartitionAwareRouting()*/));
2080+
hosts, cluster_->tablet_server(0)->cql_rpc_port(), UsePartitionAwareRouting::kFalse));
20812081
}
20822082

20832083
for (int i = 0; i <= kNumLoops; i++) {
@@ -2563,10 +2563,10 @@ class CppCassandraDriverBackpressureTest : public CppCassandraDriverTest {
25632563
return {"--tablet_server_svc_queue_length=10"s, "--max_time_in_queue_ms=-1"s};
25642564
}
25652565

2566-
bool UsePartitionAwareRouting() override {
2566+
UsePartitionAwareRouting use_partition_aware_routing() override {
25672567
// TODO: Disable partition aware routing in this test because of TSAN issue (#1837).
25682568
// Should be reenabled when issue is fixed.
2569-
return false;
2569+
return UsePartitionAwareRouting::kFalse;
25702570
}
25712571
};
25722572

@@ -2609,10 +2609,10 @@ class CppCassandraDriverTransactionalWriteTest : public CppCassandraDriverTest {
26092609
return {"--TEST_transaction_inject_flushed_delay_ms=10"s};
26102610
}
26112611

2612-
bool UsePartitionAwareRouting() override {
2612+
UsePartitionAwareRouting use_partition_aware_routing() override {
26132613
// TODO: Disable partition aware routing in this test because of TSAN issue (#1837).
26142614
// Should be reenabled when issue is fixed.
2615-
return false;
2615+
return UsePartitionAwareRouting::kFalse;
26162616
}
26172617
};
26182618

@@ -2645,10 +2645,10 @@ class CppCassandraDriverTestThreeMasters : public CppCassandraDriverTestNoPartit
26452645
return 3;
26462646
}
26472647

2648-
bool UsePartitionAwareRouting() override {
2648+
UsePartitionAwareRouting use_partition_aware_routing() override {
26492649
// TODO: Disable partition aware routing in this test because of TSAN issue (#1837).
26502650
// Should be reenabled when issue is fixed.
2651-
return false;
2651+
return UsePartitionAwareRouting::kFalse;
26522652
}
26532653
};
26542654

@@ -2753,10 +2753,10 @@ class CppCassandraDriverTestPartitionsVtableCache : public CppCassandraDriverTes
27532753
return flags;
27542754
}
27552755

2756-
bool UsePartitionAwareRouting() override {
2756+
UsePartitionAwareRouting use_partition_aware_routing() override {
27572757
// TODO: Disable partition aware routing in this test because of TSAN issue (#1837).
27582758
// Should be reenabled when issue is fixed.
2759-
return false;
2759+
return UsePartitionAwareRouting::kFalse;
27602760
}
27612761

27622762
int table_idx_ = 0;
@@ -2817,10 +2817,10 @@ class CppCassandraDriverRejectionTest : public CppCassandraDriverTest {
28172817
"--linear_backoff_ms=10"};
28182818
}
28192819

2820-
bool UsePartitionAwareRouting() override {
2820+
UsePartitionAwareRouting use_partition_aware_routing() override {
28212821
// Disable partition aware routing in this test because of TSAN issue (#1837).
28222822
// Should be reenabled when issue is fixed.
2823-
return false;
2823+
return UsePartitionAwareRouting::kFalse;
28242824
}
28252825
};
28262826

@@ -2921,10 +2921,10 @@ class CppCassandraDriverSmallSoftLimitTest : public CppCassandraDriverTest {
29212921
};
29222922
}
29232923

2924-
bool UsePartitionAwareRouting() override {
2924+
UsePartitionAwareRouting use_partition_aware_routing() override {
29252925
// Disable partition aware routing in this test because of TSAN issue (#1837).
29262926
// Should be reenabled when issue is fixed.
2927-
return false;
2927+
return UsePartitionAwareRouting::kFalse;
29282928
}
29292929
};
29302930

‎src/yb/integration-tests/cql_test_base.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ void CqlTestBase::SetUp() {
4949
ASSERT_OK(cql_server_->Start());
5050

5151
driver_ = std::make_unique<CppCassandraDriver>(
52-
std::vector<std::string>{ cql_host }, cql_port, false);
52+
std::vector<std::string>{ cql_host }, cql_port, UsePartitionAwareRouting::kFalse);
5353
}
5454

5555
void CqlTestBase::DoTearDown() {

‎src/yb/integration-tests/cql_test_util.cc‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,18 @@ CassandraRowIterator CassandraRow::CreateIterator() const {
163163
return CassandraRowIterator(cass_iterator_from_row(cass_row_));
164164
}
165165

166+
std::string CassandraRow::RenderToString(const std::string& separator) {
167+
std::string result;
168+
auto iter = CreateIterator();
169+
while (iter.Next()) {
170+
if (!result.empty()) {
171+
result += separator;
172+
}
173+
result += iter.Value().ToString();
174+
}
175+
return result;
176+
}
177+
166178
void CassandraRow::TakeIterator(CassIteratorPtr iterator) {
167179
cass_iterator_ = std::move(iterator);
168180
}
@@ -183,6 +195,20 @@ CassandraIterator CassandraResult::CreateIterator() const {
183195
return CassandraIterator(cass_iterator_from_result(cass_result_.get()));
184196
}
185197

198+
std::string CassandraResult::RenderToString(
199+
const std::string& line_separator, const std::string& value_separator) const {
200+
std::string result;
201+
auto iter = CreateIterator();
202+
while (iter.Next()) {
203+
auto row = iter.Row();
204+
if (!result.empty()) {
205+
result += ";";
206+
}
207+
result += row.RenderToString();
208+
}
209+
return result;
210+
}
211+
186212
bool CassandraFuture::Ready() const {
187213
return cass_future_ready(future_.get());
188214
}
@@ -315,6 +341,10 @@ Result<CassandraResult> CassandraSession::ExecuteWithResult(const CassandraState
315341
return future.Result();
316342
}
317343

344+
Result<std::string> CassandraSession::ExecuteAndRenderToString(const std::string& statement) {
345+
return VERIFY_RESULT(ExecuteWithResult(statement)).RenderToString();
346+
}
347+
318348
CassandraFuture CassandraSession::ExecuteGetFuture(const CassandraStatement& statement) {
319349
return CassandraFuture(
320350
cass_session_execute(cass_session_.get(), statement.cass_statement_.get()));
@@ -373,11 +403,18 @@ CassandraStatement CassandraPrepared::Bind() {
373403
const MonoDelta kCassandraTimeOut = RegularBuildVsSanitizers(12s, 60s);
374404

375405
CppCassandraDriver::CppCassandraDriver(
376-
const std::vector<std::string>& hosts, uint16_t port, bool use_partition_aware_routing) {
406+
const std::vector<std::string>& hosts, uint16_t port,
407+
UsePartitionAwareRouting use_partition_aware_routing) {
377408

378409
// Enable detailed tracing inside driver.
379410
if (VLOG_IS_ON(4)) {
380411
cass_log_set_level(CASS_LOG_TRACE);
412+
} else if (VLOG_IS_ON(3)) {
413+
cass_log_set_level(CASS_LOG_DEBUG);
414+
} else if (VLOG_IS_ON(2)) {
415+
cass_log_set_level(CASS_LOG_INFO);
416+
} else if (VLOG_IS_ON(1)) {
417+
cass_log_set_level(CASS_LOG_WARN);
381418
}
382419

383420
auto hosts_str = JoinStrings(hosts, ",");

‎src/yb/integration-tests/cql_test_util.h‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ class CassandraRow {
9797

9898
CassandraRowIterator CreateIterator() const;
9999

100+
std::string RenderToString(const std::string& separator = ",");
101+
100102
void TakeIterator(CassIteratorPtr iterator);
101103

102104
private:
@@ -127,6 +129,9 @@ class CassandraResult {
127129

128130
CassandraIterator CreateIterator() const;
129131

132+
std::string RenderToString(const std::string& line_separator = ";",
133+
const std::string& value_separator = ",") const;
134+
130135
private:
131136
CassResultPtr cass_result_;
132137
};
@@ -236,6 +241,8 @@ class CassandraSession {
236241

237242
Result<CassandraResult> ExecuteWithResult(const std::string& query);
238243

244+
Result<std::string> ExecuteAndRenderToString(const std::string& statement);
245+
239246
template <class Action>
240247
CHECKED_STATUS ExecuteAndProcessOneRow(
241248
const CassandraStatement& statement, const Action& action) {
@@ -279,10 +286,13 @@ class CassandraSession {
279286
CassSessionPtr cass_session_;
280287
};
281288

289+
YB_STRONGLY_TYPED_BOOL(UsePartitionAwareRouting);
290+
282291
class CppCassandraDriver {
283292
public:
284293
CppCassandraDriver(
285-
const std::vector<std::string>& hosts, uint16_t port, bool use_partition_aware_routing);
294+
const std::vector<std::string>& hosts, uint16_t port,
295+
UsePartitionAwareRouting use_partition_aware_routing);
286296

287297
~CppCassandraDriver();
288298

‎src/yb/tools/yb-admin-snapshot-schedule-test.cc‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include "yb/util/date_time.h"
2626
#include "yb/util/random_util.h"
27+
#include "yb/util/range.h"
2728

2829
#include "yb/yql/pgwrapper/libpq_utils.h"
2930

@@ -188,8 +189,9 @@ class YbAdminSnapshotScheduleTest : public AdminTestBase {
188189
for (int i = 0; i < cluster_->num_tablet_servers(); ++i) {
189190
hosts.push_back(cluster_->tablet_server(i)->bind_host());
190191
}
192+
LOG(INFO) << "CQL hosts: " << AsString(hosts);
191193
cql_driver_ = std::make_unique<CppCassandraDriver>(
192-
hosts, cluster_->tablet_server(0)->cql_rpc_port(), true);
194+
hosts, cluster_->tablet_server(0)->cql_rpc_port(), UsePartitionAwareRouting::kTrue);
193195
}
194196
auto result = VERIFY_RESULT(cql_driver_->CreateSession());
195197
if (!db_name.empty()) {
@@ -212,6 +214,11 @@ class YbAdminSnapshotScheduleTest : public AdminTestBase {
212214

213215
void TestUndeleteTable(bool restart_masters);
214216

217+
void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override {
218+
options->bind_to_unique_loopback_addresses = true;
219+
options->use_same_ts_ports = true;
220+
}
221+
215222
std::unique_ptr<CppCassandraDriver> cql_driver_;
216223
};
217224

@@ -430,5 +437,55 @@ TEST_F(YbAdminSnapshotScheduleTest, UndeleteIndex) {
430437
ASSERT_EQ(res, 1);
431438
}
432439

440+
// This test is for schema version patching after restore.
441+
// Consider the following scenario, w/o patching:
442+
//
443+
// 1) Create table.
444+
// 2) Add text column to table. Schema version - 1.
445+
// 3) Insert values into table. Each CQL proxy suppose schema version 1 for this table.
446+
// 4) Restore to time between (1) and (2). Schema version - 0.
447+
// 5) Add int column to table. Schema version - 1.
448+
// 6) Try insert values with wrong type into table.
449+
//
450+
// So table has schema version 1, but new column is INT.
451+
// CQL proxy suppose schema version is also 1, but the last column is TEXT.
452+
TEST_F(YbAdminSnapshotScheduleTest, AlterTable) {
453+
const auto kKeys = Range(10);
454+
455+
auto schedule_id = ASSERT_RESULT(PrepareCql());
456+
457+
auto conn = ASSERT_RESULT(CqlConnect(client::kTableName.namespace_name()));
458+
459+
ASSERT_OK(conn.ExecuteQuery(
460+
"CREATE TABLE test_table (key INT PRIMARY KEY, value TEXT)"));
461+
462+
for (auto key : kKeys) {
463+
ASSERT_OK(conn.ExecuteQuery(Format(
464+
"INSERT INTO test_table (key, value) VALUES ($0, 'A')", key)));
465+
}
466+
467+
Timestamp time(ASSERT_RESULT(WallClock()->Now()).time_point);
468+
469+
ASSERT_OK(conn.ExecuteQuery(
470+
"ALTER TABLE test_table ADD value2 TEXT"));
471+
472+
for (auto key : kKeys) {
473+
ASSERT_OK(conn.ExecuteQuery(Format(
474+
"INSERT INTO test_table (key, value, value2) VALUES ($0, 'B', 'X')", key)));
475+
}
476+
477+
ASSERT_OK(RestoreSnapshotSchedule(schedule_id, time));
478+
479+
ASSERT_OK(conn.ExecuteQuery(
480+
"ALTER TABLE test_table ADD value2 INT"));
481+
482+
for (auto key : kKeys) {
483+
// It would succeed on some TServers if we would not refresh metadata after restore.
484+
// But it should not succeed because of last column type.
485+
ASSERT_NOK(conn.ExecuteQuery(Format(
486+
"INSERT INTO test_table (key, value, value2) VALUES ($0, 'D', 'Y')", key)));
487+
}
488+
}
489+
433490
} // namespace tools
434491
} // namespace yb

0 commit comments

Comments
 (0)