Skip to content

Commit 9fa72e7

Browse files
committed
[#27832] docdb: TServers try to relinquish their ysql lease on receiving SIGTERM
Summary: On the shutdown path in tservers, make a best-effort attempt to release the ysql lease if one is currently held. This allows DDLs and DMLs on other tservers to proceed without waiting for the shut down TServer's ysql lease to expire normally. **Upgrade/Rollback safety:** Proto changes: - adds `lease_relinquished` to `SysObjectLockEntryPB::lease_info`. This field defaults to `false`, we expect upgraded servers to interpret its absence as `false`. - adds a new RPC, `RelinquishYsqlLease`, to the `MasterDdlService` Test Plan: ``` ./yb_build.sh release --cxx-test master-test --gtest_filter 'MasterTest.RelinquishLease*' ./yb_build.sh release --cxx-test object_lock-test --gtest_filter 'ExternalObjectLockTestLongLeaseTTL.SigTerm' ``` Reviewers: amitanand, bkolagani Reviewed By: amitanand Subscribers: slingam, rthallam, ybase Differential Revision: https://phorge.dev.yugabyte.com/D45605
1 parent 96ec8a8 commit 9fa72e7

21 files changed

+315
-65
lines changed

src/yb/integration-tests/object_lock-test.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,29 @@ TEST_F(ExternalObjectLockTest, RefreshYsqlLease) {
11351135
ASSERT_FALSE(info.has_ddl_lock_entries());
11361136
}
11371137

1138+
class ExternalObjectLockTestLongLeaseTTL : public ExternalObjectLockTest {
1139+
ExternalMiniClusterOptions MakeExternalMiniClusterOptions() override;
1140+
int ReplicationFactor() override;
1141+
size_t NumberOfTabletServers() override;
1142+
};
1143+
1144+
TEST_F(ExternalObjectLockTestLongLeaseTTL, SigTerm) {
1145+
auto kLeaseTimeoutDeadline = MonoDelta::FromSeconds(10);
1146+
ASSERT_LT(
1147+
kLeaseTimeoutDeadline.ToMilliseconds(),
1148+
std::stoll(ASSERT_RESULT(cluster_->GetFlag(
1149+
ASSERT_NOTNULL(cluster_->GetLeaderMaster()), "master_ysql_operation_lease_ttl_ms"))));
1150+
auto ts = tablet_server(0);
1151+
auto ts_uuid = ts->uuid();
1152+
ts->Shutdown(SafeShutdown::kTrue);
1153+
// TServer should lose its lease before the lease TTL.
1154+
// Use EXPECT here so we restart the TS in case of failure.
1155+
EXPECT_OK(WaitForTServerLeaseToExpire(ts_uuid, kLeaseTimeoutDeadline));
1156+
ASSERT_OK(ts->Restart());
1157+
// TServer should be able to re-acquire the lease after it restarts.
1158+
ASSERT_OK(WaitForTServerLease(ts_uuid, 10s));
1159+
}
1160+
11381161
class MultiMasterObjectLockTest : public ObjectLockTest {
11391162
protected:
11401163
int num_masters() override { return 3; }
@@ -1731,4 +1754,24 @@ ExternalObjectLockTestOneTSWithoutLease::MakeExternalMiniClusterOptions() {
17311754
return opts;
17321755
}
17331756

1757+
ExternalMiniClusterOptions ExternalObjectLockTestLongLeaseTTL::MakeExternalMiniClusterOptions() {
1758+
ExternalMiniClusterOptions opts;
1759+
opts.num_tablet_servers = NumberOfTabletServers();
1760+
opts.replication_factor = ReplicationFactor();
1761+
opts.enable_ysql = true;
1762+
opts.extra_master_flags = {
1763+
Format("--master_ysql_operation_lease_ttl_ms=$0", 20 * 1000),
1764+
"--enable_load_balancing=false"};
1765+
opts.extra_tserver_flags = {
1766+
Format("--allowed_preview_flags_csv=$0,$1",
1767+
"enable_object_locking_for_table_locks", "ysql_yb_ddl_transaction_block_enabled"),
1768+
"--enable_object_locking_for_table_locks",
1769+
"--ysql_yb_ddl_transaction_block_enabled",
1770+
Format("--ysql_lease_refresher_interval_ms=$0", kDefaultYSQLLeaseRefreshIntervalMilli)};
1771+
return opts;
1772+
}
1773+
1774+
int ExternalObjectLockTestLongLeaseTTL::ReplicationFactor() { return 1; }
1775+
size_t ExternalObjectLockTestLongLeaseTTL::NumberOfTabletServers() { return 1; }
1776+
17341777
} // namespace yb

src/yb/master/catalog_entity_info.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1360,9 +1360,24 @@ std::string DdlLogEntry::id() const {
13601360
// ObjectLockInfo
13611361
// ================================================================================================
13621362

1363-
std::variant<ObjectLockInfo::WriteLock, SysObjectLockEntryPB::LeaseInfoPB>
1363+
Result<std::variant<ObjectLockInfo::WriteLock, SysObjectLockEntryPB::LeaseInfoPB>>
13641364
ObjectLockInfo::RefreshYsqlOperationLease(const NodeInstancePB& instance, MonoDelta lease_ttl) {
13651365
auto l = LockForWrite();
1366+
auto& current_lease_info = l->pb.lease_info();
1367+
if (instance.instance_seqno() < current_lease_info.instance_seqno()) {
1368+
return STATUS_FORMAT(
1369+
IllegalState,
1370+
"Cannot grant lease, instance seqno of requestor $0 is lower than instance seqno of a "
1371+
"previously granted lease $1",
1372+
instance.instance_seqno(), current_lease_info.instance_seqno());
1373+
}
1374+
if (current_lease_info.lease_relinquished() &&
1375+
instance.instance_seqno() <= current_lease_info.instance_seqno()) {
1376+
return STATUS_FORMAT(
1377+
IllegalState,
1378+
"Cannot grant lease, lease has been relinquished by instance_seqno $0 already",
1379+
current_lease_info.instance_seqno());
1380+
}
13661381
{
13671382
std::lock_guard l(mutex_);
13681383
// When doing this mutation we cannot be sure the tserver receives the response.
@@ -1377,6 +1392,7 @@ ObjectLockInfo::RefreshYsqlOperationLease(const NodeInstancePB& instance, MonoDe
13771392
lease_info.set_live_lease(true);
13781393
lease_info.set_lease_epoch(lease_info.lease_epoch() + 1);
13791394
lease_info.set_instance_seqno(instance.instance_seqno());
1395+
lease_info.set_lease_relinquished(false);
13801396
return std::move(l);
13811397
}
13821398

src/yb/master/catalog_entity_info.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ class ObjectLockInfo : public MetadataCowWrapper<PersistentObjectLockInfo> {
11021102
// Return the user defined type's ID. Does not require synchronization.
11031103
virtual const std::string& id() const override { return ts_uuid_; }
11041104

1105-
std::variant<ObjectLockInfo::WriteLock, SysObjectLockEntryPB::LeaseInfoPB>
1105+
Result<std::variant<ObjectLockInfo::WriteLock, SysObjectLockEntryPB::LeaseInfoPB>>
11061106
RefreshYsqlOperationLease(const NodeInstancePB& instance, MonoDelta lease_ttl) EXCLUDES(mutex_);
11071107

11081108
virtual void Load(const SysObjectLockEntryPB& metadata) override;

src/yb/master/catalog_entity_info.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,8 @@ message SysObjectLockEntryPB {
452452

453453
// The instance_seqno of the tserver process to which this lease was granted.
454454
optional int64 instance_seqno = 3;
455+
456+
optional bool lease_relinquished = 4;
455457
}
456458
optional LeaseInfoPB lease_info = 2;
457459
map<uint64, tserver.ReleaseObjectLockRequestPB> in_progress_release_request = 3;

src/yb/master/catalog_manager.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5957,6 +5957,13 @@ Status CatalogManager::RefreshYsqlLease(const RefreshYsqlLeaseRequestPB* req,
59575957
return object_lock_info_manager_->RefreshYsqlLease(*req, *resp, *rpc, epoch);
59585958
}
59595959

5960+
Status CatalogManager::RelinquishYsqlLease(const RelinquishYsqlLeaseRequestPB* req,
5961+
RelinquishYsqlLeaseResponsePB* resp,
5962+
rpc::RpcContext* rpc,
5963+
const LeaderEpoch& epoch) {
5964+
return object_lock_info_manager_->RelinquishYsqlLease(*req, *resp, *rpc, epoch);
5965+
}
5966+
59605967
Status CatalogManager::TruncateTable(const TableId& table_id,
59615968
TruncateTableResponsePB* resp,
59625969
rpc::RpcContext* rpc,

src/yb/master/catalog_manager.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,14 +382,19 @@ class CatalogManager : public CatalogManagerIf, public SnapshotCoordinatorContex
382382
rpc::RpcContext* rpc,
383383
const LeaderEpoch& epoch);
384384

385+
// Get the information about an in-progress truncate operation.
386+
Status IsTruncateTableDone(const IsTruncateTableDoneRequestPB* req,
387+
IsTruncateTableDoneResponsePB* resp);
388+
385389
Status RefreshYsqlLease(const RefreshYsqlLeaseRequestPB* req,
386390
RefreshYsqlLeaseResponsePB* resp,
387391
rpc::RpcContext* rpc,
388392
const LeaderEpoch& epoch);
389393

390-
// Get the information about an in-progress truncate operation.
391-
Status IsTruncateTableDone(const IsTruncateTableDoneRequestPB* req,
392-
IsTruncateTableDoneResponsePB* resp);
394+
Status RelinquishYsqlLease(const RelinquishYsqlLeaseRequestPB* req,
395+
RelinquishYsqlLeaseResponsePB* resp,
396+
rpc::RpcContext* rpc,
397+
const LeaderEpoch& epoch);
393398

394399
// Backfill the specified index. Currently only supported for YSQL. YCQL does not need this as
395400
// master automatically runs backfill according to the DocDB permissions.

src/yb/master/master-test.cc

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ class MasterTest : public MasterTestBase {
127127
Result<scoped_refptr<NamespaceInfo>> FindNamespaceByName(
128128
YQLDatabase db_type, const std::string& name);
129129

130-
Result<TSHeartbeatResponsePB> SendNewTSRegistrationHeartbeat(const std::string& uuid);
130+
Result<TSHeartbeatResponsePB> SendNewTSRegistrationHeartbeat(
131+
const std::string& uuid, int64_t instance_seqno);
131132

132133
private:
133134
// Used by SendNewTSRegistrationHeartbeat to avoid host port collisions.
@@ -2775,16 +2776,16 @@ TEST_F(MasterTest, RefreshYsqlLeaseWithoutRegistration) {
27752776

27762777
TEST_F(MasterTest, RefreshYsqlLease) {
27772778
ANNOTATE_UNPROTECTED_WRITE(FLAGS_enable_ysql_operation_lease) = true;
2778-
const std::string kTsUUID1 = "ts-uuid1";
2779-
const std::string kTsUUID2 = "ts-uuid2";
2779+
const std::string kTsUUID = "ts-uuid1";
2780+
constexpr uint64_t kSeqno = 1;
27802781

2781-
auto reg_resp1 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID1));
2782+
auto reg_resp1 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID, kSeqno));
27822783
ASSERT_FALSE(reg_resp1.needs_reregister());
27832784

27842785
auto ddl_client = MasterDDLClient{std::move(*proxy_ddl_)};
27852786
auto lease_refresh_send_time_ms = MonoTime::Now().GetDeltaSinceMin().ToMilliseconds();
2786-
auto info = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2787-
kTsUUID1, /* instance_seqno */ 1, lease_refresh_send_time_ms, {}));
2787+
auto info =
2788+
ASSERT_RESULT(ddl_client.RefreshYsqlLease(kTsUUID, kSeqno, lease_refresh_send_time_ms, {}));
27882789
ASSERT_TRUE(info.new_lease());
27892790
ASSERT_EQ(info.lease_epoch(), 1);
27902791
ASSERT_GT(
@@ -2794,27 +2795,82 @@ TEST_F(MasterTest, RefreshYsqlLease) {
27942795
// todo(zdrudi): but we need to do this and check the bootstrap entries...
27952796
// Refresh lease again. Since we omitted current lease epoch, master leader should still say this
27962797
// is a new lease.
2797-
info = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2798-
kTsUUID1, /* instance_seqno */ 1, lease_refresh_send_time_ms, {}));
2798+
info =
2799+
ASSERT_RESULT(ddl_client.RefreshYsqlLease(kTsUUID, kSeqno, lease_refresh_send_time_ms, {}));
27992800
ASSERT_TRUE(info.new_lease());
28002801
ASSERT_EQ(info.lease_epoch(), 1);
28012802
ASSERT_GT(info.lease_expiry_time_ms(), lease_refresh_send_time_ms);
28022803

28032804
// Refresh lease again. We included current lease epoch but it's incorrect.
2804-
info = ASSERT_RESULT(
2805-
ddl_client.RefreshYsqlLease(kTsUUID1, /* instance_seqno */ 1, lease_refresh_send_time_ms, 0));
2805+
info = ASSERT_RESULT(ddl_client.RefreshYsqlLease(kTsUUID, kSeqno, lease_refresh_send_time_ms, 0));
28062806
ASSERT_TRUE(info.new_lease());
28072807
ASSERT_EQ(info.lease_epoch(), 1);
28082808
ASSERT_GT(info.lease_expiry_time_ms(), lease_refresh_send_time_ms);
28092809

28102810
// Refresh lease again. Current lease epoch is correct so master leader should not set new lease
28112811
// bit.
2812-
info = ASSERT_RESULT(
2813-
ddl_client.RefreshYsqlLease(kTsUUID1, /* instance_seqno */ 1, lease_refresh_send_time_ms, 1));
2812+
info = ASSERT_RESULT(ddl_client.RefreshYsqlLease(kTsUUID, kSeqno, lease_refresh_send_time_ms, 1));
28142813
ASSERT_FALSE(info.new_lease());
28152814
ASSERT_GT(info.lease_expiry_time_ms(), lease_refresh_send_time_ms);
28162815
}
28172816

2817+
TEST_F(MasterTest, RelinquishLease) {
2818+
ANNOTATE_UNPROTECTED_WRITE(FLAGS_enable_ysql_operation_lease) = true;
2819+
const std::string kTsUUID = "ts-uuid1";
2820+
constexpr uint64_t kSeqno1 = 1;
2821+
auto reg_resp1 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID, kSeqno1));
2822+
ASSERT_FALSE(reg_resp1.needs_reregister());
2823+
2824+
auto ddl_client = MasterDDLClient{std::move(*proxy_ddl_)};
2825+
auto info1 = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2826+
kTsUUID, kSeqno1, MonoTime::Now().GetDeltaSinceMin().ToMilliseconds(), {}));
2827+
ASSERT_TRUE(info1.new_lease());
2828+
ASSERT_EQ(info1.lease_epoch(), 1);
2829+
2830+
ASSERT_OK(ddl_client.RelinquishYsqlLease(kTsUUID, kSeqno1));
2831+
auto list_ts = ASSERT_RESULT(cluster_client_->ListTabletServers());
2832+
ASSERT_EQ(list_ts.servers_size(), 1);
2833+
ASSERT_FALSE(list_ts.servers(0).lease_info().is_live());
2834+
2835+
// We should be able to get a new lease for a new instance of the ts.
2836+
constexpr uint64_t kSeqno2 = 2;
2837+
auto reg_resp2 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID, kSeqno2));
2838+
ASSERT_FALSE(reg_resp2.needs_reregister());
2839+
auto info2 = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2840+
kTsUUID, kSeqno2, MonoTime::Now().GetDeltaSinceMin().ToMilliseconds(), {}));
2841+
ASSERT_TRUE(info2.new_lease());
2842+
ASSERT_EQ(info2.lease_epoch(), 2);
2843+
}
2844+
2845+
TEST_F(MasterTest, RelinquishLeaseOfReplacedTS) {
2846+
ANNOTATE_UNPROTECTED_WRITE(FLAGS_enable_ysql_operation_lease) = true;
2847+
const std::string kTsUUID = "ts-uuid1";
2848+
constexpr uint64_t kSeqno1 = 1;
2849+
auto reg_resp1 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID, kSeqno1));
2850+
ASSERT_FALSE(reg_resp1.needs_reregister());
2851+
2852+
auto ddl_client = MasterDDLClient{std::move(*proxy_ddl_)};
2853+
auto info1 = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2854+
kTsUUID, kSeqno1, MonoTime::Now().GetDeltaSinceMin().ToMilliseconds(), {}));
2855+
ASSERT_TRUE(info1.new_lease());
2856+
ASSERT_EQ(info1.lease_epoch(), 1);
2857+
2858+
// Simulate a second ts process replacing the first.
2859+
constexpr uint64_t kSeqno2 = 2;
2860+
auto reg_resp2 = ASSERT_RESULT(SendNewTSRegistrationHeartbeat(kTsUUID, kSeqno2));
2861+
ASSERT_FALSE(reg_resp2.needs_reregister());
2862+
auto info2 = ASSERT_RESULT(ddl_client.RefreshYsqlLease(
2863+
kTsUUID, kSeqno2, MonoTime::Now().GetDeltaSinceMin().ToMilliseconds(), {}));
2864+
ASSERT_TRUE(info2.new_lease());
2865+
ASSERT_EQ(info2.lease_epoch(), 2);
2866+
2867+
// Send relinquish lease request from the first ts instance.
2868+
auto status = ddl_client.RelinquishYsqlLease(kTsUUID, kSeqno1);
2869+
ASSERT_NOK(status);
2870+
ASSERT_STR_CONTAINS(
2871+
status.ToString(), "Relinquish lease request for a replaced tserver instance");
2872+
}
2873+
28182874
Result<TSHeartbeatResponsePB> MasterTest::SendHeartbeat(
28192875
TSToMasterCommonPB common, std::optional<TSRegistrationPB> registration,
28202876
std::optional<TabletReportPB> report) {
@@ -2839,7 +2895,8 @@ Result<TSHeartbeatResponsePB> MasterTest::SendHeartbeat(
28392895
return resp;
28402896
}
28412897

2842-
Result<TSHeartbeatResponsePB> MasterTest::SendNewTSRegistrationHeartbeat(const std::string& uuid) {
2898+
Result<TSHeartbeatResponsePB> MasterTest::SendNewTSRegistrationHeartbeat(
2899+
const std::string& uuid, int64_t instance_seqno) {
28432900
TSRegistrationPB reg;
28442901
*reg.mutable_common()->add_private_rpc_addresses() =
28452902
MakeHostPortPB("localhost", 1000 + registered_ts_count_);
@@ -2849,7 +2906,7 @@ Result<TSHeartbeatResponsePB> MasterTest::SendNewTSRegistrationHeartbeat(const s
28492906

28502907
TSToMasterCommonPB common;
28512908
common.mutable_ts_instance()->set_permanent_uuid(uuid);
2852-
common.mutable_ts_instance()->set_instance_seqno(1);
2909+
common.mutable_ts_instance()->set_instance_seqno(instance_seqno);
28532910
auto result = SendHeartbeat(common, reg);
28542911
if (result.ok()) {
28552912
registered_ts_count_++;

src/yb/master/master_ddl.proto

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,14 @@ message RefreshYsqlLeaseResponsePB {
864864
}
865865
}
866866

867+
message RelinquishYsqlLeaseRequestPB {
868+
optional NodeInstancePB instance = 1;
869+
}
870+
871+
message RelinquishYsqlLeaseResponsePB {
872+
optional MasterErrorPB error = 1;
873+
}
874+
867875
service MasterDdl {
868876
option (yb.rpc.custom_service_name) = "yb.master.MasterService";
869877

@@ -928,8 +936,9 @@ service MasterDdl {
928936
option (yb.rpc.send_metadata) = true;
929937
}
930938
rpc RefreshYsqlLease(RefreshYsqlLeaseRequestPB)
931-
returns (RefreshYsqlLeaseResponsePB);
932-
939+
returns (RefreshYsqlLeaseResponsePB);
940+
rpc RelinquishYsqlLease(RelinquishYsqlLeaseRequestPB)
941+
returns (RelinquishYsqlLeaseResponsePB);
933942
rpc GetObjectLockStatus(GetObjectLockStatusRequestPB)
934943
returns (GetObjectLockStatusResponsePB);
935944
}

src/yb/master/master_ddl_client.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,17 @@ Result<RefreshYsqlLeaseInfoPB> MasterDDLClient::RefreshYsqlLease(
9393
return resp.info();
9494
}
9595

96+
Status MasterDDLClient::RelinquishYsqlLease(
97+
const std::string& permanent_uuid, int64_t instance_seqno) {
98+
RelinquishYsqlLeaseRequestPB req;
99+
req.mutable_instance()->set_permanent_uuid(permanent_uuid);
100+
req.mutable_instance()->set_instance_seqno(instance_seqno);
101+
RelinquishYsqlLeaseResponsePB resp;
102+
rpc::RpcController rpc;
103+
RETURN_NOT_OK(proxy_.RelinquishYsqlLease(req, &resp, &rpc));
104+
return ResponseStatus(resp);
105+
}
106+
96107
Status MasterDDLClient::DeleteTable(const TableId& id, MonoDelta timeout) {
97108
DeleteTableRequestPB req;
98109
req.mutable_table()->set_table_id(id);

src/yb/master/master_ddl_client.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ class MasterDDLClient {
4040
const std::string& permanent_uuid, int64_t instance_seqno, uint64_t time_ms,
4141
std::optional<uint64_t> current_lease_epoch);
4242

43+
Status RelinquishYsqlLease(const std::string& permanent_uuid, int64_t instance_seqno);
44+
4345
Status DeleteTable(const TableId& id, MonoDelta timeout);
4446

4547
private:

0 commit comments

Comments
 (0)