Skip to content

Commit 2043d88

Browse files
azevaykinazevaykin
committed
Lock index impl tables before scanning them (#17229)
1 parent 27039c5 commit 2043d88

13 files changed

+339
-120
lines changed

‎ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp

Lines changed: 158 additions & 0 deletions
Large diffs are not rendered by default.

‎ydb/core/protos/flat_scheme_op.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,7 @@ message TIndexBuildControl {
15801580

15811581
message TLockConfig {
15821582
optional string Name = 1;
1583+
optional uint64 LockTxId = 2; // if missing, current tx id is used
15831584
}
15841585

15851586
message TLockGuard {

‎ydb/core/tx/schemeshard/schemeshard__operation_apply_build_index.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ ISubOperation::TPtr FinalizeIndexImplTable(TOperationContext& context, const TPa
3030
return CreateFinalizeBuildIndexImplTable(partId, transaction);
3131
}
3232

33-
ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& nextId, const TOperationId& partId, const TString& name, const TPathId& pathId, bool& rejected) {
33+
ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& nextId, const TOperationId& partId, const TString& name, const TPathId& pathId, const NKikimrSchemeOp::TLockGuard& lockGuard, bool& rejected) {
3434
TPath implTable = index.Child(name);
3535
Y_ABORT_UNLESS(implTable->PathId == pathId);
3636
Y_ABORT_UNLESS(implTable.LeafName() == name);
@@ -48,6 +48,11 @@ ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& n
4848
}
4949
rejected = false;
5050
auto transaction = TransactionTemplate(index.PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpDropTable);
51+
if (implTable.IsLocked()) {
52+
// because some impl tables may be not locked, do not pass lock guard for them
53+
// otherwise `CheckLocks` check would fail
54+
*transaction.MutableLockGuard() = lockGuard;
55+
}
5156
auto operation = transaction.MutableDrop();
5257
operation->SetName(name);
5358
return CreateDropTable(partId, transaction);
@@ -98,7 +103,7 @@ TVector<ISubOperation::TPtr> ApplyBuildIndex(TOperationId nextId, const TTxTrans
98103
const auto partId = NextPartId(nextId, result);
99104
if (NTableIndex::IsBuildImplTable(indexImplTableName)) {
100105
bool rejected = false;
101-
auto op = DropIndexImplTable(index, nextId, partId, indexImplTableName, indexChildItems.second, rejected);
106+
auto op = DropIndexImplTable(index, nextId, partId, indexImplTableName, indexChildItems.second, tx.GetLockGuard(), rejected);
102107
if (rejected) {
103108
return {std::move(op)};
104109
}
@@ -153,7 +158,7 @@ TVector<ISubOperation::TPtr> CancelBuildIndex(TOperationId nextId, const TTxTran
153158
for (auto& indexChildItems : index.Base()->GetChildren()) {
154159
const auto partId = NextPartId(nextId, result);
155160
bool rejected = false;
156-
auto op = DropIndexImplTable(index, nextId, partId, indexChildItems.first, indexChildItems.second, rejected);
161+
auto op = DropIndexImplTable(index, nextId, partId, indexChildItems.first, indexChildItems.second, tx.GetLockGuard(), rejected);
157162
if (rejected) {
158163
return {std::move(op)};
159164
}

‎ydb/core/tx/schemeshard/schemeshard__operation_create_lock.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ class TCreateLock: public TSubOperation {
9595
THolder<TProposeResponse> Propose(const TString&, TOperationContext& context) override {
9696
const auto& workingDir = Transaction.GetWorkingDir();
9797
const auto& op = Transaction.GetLockConfig();
98+
const TTxId lockTxId = op.HasLockTxId()
99+
? TTxId(op.GetLockTxId())
100+
: OperationId.GetTxId();
98101

99102
LOG_N("TCreateLock Propose"
100103
<< ": opId# " << OperationId
@@ -158,13 +161,12 @@ class TCreateLock: public TSubOperation {
158161
const auto pathId = tablePath.Base()->PathId;
159162
result->SetPathId(pathId.LocalPathId);
160163

161-
if (tablePath.LockedBy() == OperationId.GetTxId()) {
164+
if (tablePath.LockedBy() == lockTxId) {
162165
result->SetError(NKikimrScheme::StatusAlreadyExists, TStringBuilder() << "path checks failed"
163166
<< ", path already locked by this operation"
164167
<< ", path: " << tablePath.PathString());
165168
return result;
166169
}
167-
168170
TString errStr;
169171
if (!context.SS->CheckLocks(pathId, Transaction, errStr)) {
170172
result->SetError(NKikimrScheme::StatusMultipleModifications, errStr);
@@ -177,7 +179,7 @@ class TCreateLock: public TSubOperation {
177179
context.MemChanges.GrabNewTxState(context.SS, OperationId);
178180

179181
context.DbChanges.PersistPath(pathId);
180-
context.DbChanges.PersistLongLock(pathId, OperationId.GetTxId());
182+
context.DbChanges.PersistLongLock(pathId, lockTxId);
181183
context.DbChanges.PersistTxState(OperationId);
182184

183185
Y_ABORT_UNLESS(!context.SS->FindTx(OperationId));
@@ -194,7 +196,7 @@ class TCreateLock: public TSubOperation {
194196
context.OnComplete.Dependence(splitOpId.GetTxId(), OperationId.GetTxId());
195197
}
196198

197-
context.SS->LockedPaths[pathId] = OperationId.GetTxId();
199+
context.SS->LockedPaths[pathId] = lockTxId;
198200
context.SS->TabletCounters->Simple()[COUNTER_LOCKS_COUNT].Add(1);
199201

200202
context.OnComplete.ActivateTx(OperationId);

‎ydb/core/tx/schemeshard/schemeshard__operation_drop_lock.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ class TDropLock: public TSubOperation {
164164
const auto pathId = dstPath.Base()->PathId;
165165
result->SetPathId(pathId.LocalPathId);
166166

167-
if (!dstPath.LockedBy()) {
167+
if (!dstPath.IsLocked()) {
168168
result->SetError(TEvSchemeShard::EStatus::StatusAlreadyExists, TStringBuilder() << "path checks failed"
169169
<< ", path already unlocked"
170170
<< ", path: " << dstPath.PathString());

‎ydb/core/tx/schemeshard/schemeshard__table_stats.cpp

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -475,22 +475,18 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
475475
return true;
476476
}
477477

478-
{
479-
auto path = TPath::Init(pathId, Self);
480-
auto checks = path.Check();
481-
482-
constexpr ui64 deltaShards = 2;
483-
checks
484-
.PathShardsLimit(deltaShards)
485-
.ShardsLimit(deltaShards);
486-
487-
if (!checks) {
488-
LOG_NOTICE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
489-
"Do not request full stats from datashard"
490-
<< ", datashard: " << datashardId
491-
<< ", reason: " << checks.GetError());
492-
return true;
493-
}
478+
auto path = TPath::Init(pathId, Self);
479+
auto checks = path.Check();
480+
constexpr ui64 deltaShards = 2;
481+
checks
482+
.PathShardsLimit(deltaShards)
483+
.ShardsLimit(deltaShards);
484+
if (!checks) {
485+
LOG_NOTICE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
486+
"Do not request full stats from datashard"
487+
<< ", datashard: " << datashardId
488+
<< ", reason: " << checks.GetError());
489+
return true;
494490
}
495491

496492
if (newStats.HasBorrowedData) {
@@ -500,6 +496,12 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
500496
return true;
501497
}
502498

499+
if (path.IsLocked()) {
500+
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
501+
"Postpone split tablet " << datashardId << " because it is locked by " << path.LockedBy());
502+
return true;
503+
}
504+
503505
// Request histograms from the datashard
504506
LOG_DEBUG(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
505507
"Requesting full stats from datashard %" PRIu64, rec.GetDatashardId());

‎ydb/core/tx/schemeshard/schemeshard__table_stats_histogram.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ bool TTxPartitionHistogram::Execute(TTransactionContext& txc, const TActorContex
360360
return true;
361361

362362
TTableInfo::TPtr table = Self->Tables[tableId];
363+
auto path = TPath::Init(tableId, Self);
363364

364365
if (!Self->TabletIdToShardIdx.contains(datashardId))
365366
return true;
@@ -368,6 +369,12 @@ bool TTxPartitionHistogram::Execute(TTransactionContext& txc, const TActorContex
368369
if (table->IsBackup)
369370
return true;
370371

372+
if (path.IsLocked()) {
373+
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
374+
"TTxPartitionHistogram Skip locked table tablet " << datashardId << " by " << path.LockedBy());
375+
return true;
376+
}
377+
371378
auto shardIdx = Self->TabletIdToShardIdx[datashardId];
372379
const auto forceShardSplitSettings = Self->SplitSettings.GetForceShardSplitSettings();
373380

0 commit comments

Comments
 (0)