Skip to content

Lock index impl tables before scanning them #17229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions ydb/core/protos/flat_scheme_op.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,7 @@ message TIndexBuildControl {

message TLockConfig {
optional string Name = 1;
optional uint64 LockTxId = 2; // if missing, current tx id is used
}

message TLockGuard {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ISubOperation::TPtr FinalizeIndexImplTable(TOperationContext& context, const TPa
return CreateFinalizeBuildIndexImplTable(partId, transaction);
}

ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& nextId, const TOperationId& partId, const TString& name, const TPathId& pathId, bool& rejected) {
ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& nextId, const TOperationId& partId, const TString& name, const TPathId& pathId, const NKikimrSchemeOp::TLockGuard& lockGuard, bool& rejected) {
TPath implTable = index.Child(name);
Y_ABORT_UNLESS(implTable->PathId == pathId);
Y_ABORT_UNLESS(implTable.LeafName() == name);
Expand All @@ -48,6 +48,11 @@ ISubOperation::TPtr DropIndexImplTable(const TPath& index, const TOperationId& n
}
rejected = false;
auto transaction = TransactionTemplate(index.PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpDropTable);
if (implTable.IsLocked()) {
// because some impl tables may be not locked, do not pass lock guard for them
// otherwise `CheckLocks` check would fail
*transaction.MutableLockGuard() = lockGuard;
}
auto operation = transaction.MutableDrop();
operation->SetName(name);
return CreateDropTable(partId, transaction);
Expand Down Expand Up @@ -98,7 +103,7 @@ TVector<ISubOperation::TPtr> ApplyBuildIndex(TOperationId nextId, const TTxTrans
const auto partId = NextPartId(nextId, result);
if (NTableIndex::IsBuildImplTable(indexImplTableName)) {
bool rejected = false;
auto op = DropIndexImplTable(index, nextId, partId, indexImplTableName, indexChildItems.second, rejected);
auto op = DropIndexImplTable(index, nextId, partId, indexImplTableName, indexChildItems.second, tx.GetLockGuard(), rejected);
if (rejected) {
return {std::move(op)};
}
Expand Down Expand Up @@ -153,7 +158,7 @@ TVector<ISubOperation::TPtr> CancelBuildIndex(TOperationId nextId, const TTxTran
for (auto& indexChildItems : index.Base()->GetChildren()) {
const auto partId = NextPartId(nextId, result);
bool rejected = false;
auto op = DropIndexImplTable(index, nextId, partId, indexChildItems.first, indexChildItems.second, rejected);
auto op = DropIndexImplTable(index, nextId, partId, indexChildItems.first, indexChildItems.second, tx.GetLockGuard(), rejected);
if (rejected) {
return {std::move(op)};
}
Expand Down
10 changes: 6 additions & 4 deletions ydb/core/tx/schemeshard/schemeshard__operation_create_lock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class TCreateLock: public TSubOperation {
THolder<TProposeResponse> Propose(const TString&, TOperationContext& context) override {
const auto& workingDir = Transaction.GetWorkingDir();
const auto& op = Transaction.GetLockConfig();
const TTxId lockTxId = op.HasLockTxId()
? TTxId(op.GetLockTxId())
: OperationId.GetTxId();

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

if (tablePath.LockedBy() == OperationId.GetTxId()) {
if (tablePath.LockedBy() == lockTxId) {
result->SetError(NKikimrScheme::StatusAlreadyExists, TStringBuilder() << "path checks failed"
<< ", path already locked by this operation"
<< ", path: " << tablePath.PathString());
return result;
}

TString errStr;
if (!context.SS->CheckLocks(pathId, Transaction, errStr)) {
result->SetError(NKikimrScheme::StatusMultipleModifications, errStr);
Expand All @@ -177,7 +179,7 @@ class TCreateLock: public TSubOperation {
context.MemChanges.GrabNewTxState(context.SS, OperationId);

context.DbChanges.PersistPath(pathId);
context.DbChanges.PersistLongLock(pathId, OperationId.GetTxId());
context.DbChanges.PersistLongLock(pathId, lockTxId);
context.DbChanges.PersistTxState(OperationId);

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

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

context.OnComplete.ActivateTx(OperationId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class TDropLock: public TSubOperation {
const auto pathId = dstPath.Base()->PathId;
result->SetPathId(pathId.LocalPathId);

if (!dstPath.LockedBy()) {
if (!dstPath.IsLocked()) {
result->SetError(TEvSchemeShard::EStatus::StatusAlreadyExists, TStringBuilder() << "path checks failed"
<< ", path already unlocked"
<< ", path: " << dstPath.PathString());
Expand Down
34 changes: 18 additions & 16 deletions ydb/core/tx/schemeshard/schemeshard__table_stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,22 +484,18 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
return true;
}

{
auto path = TPath::Init(pathId, Self);
auto checks = path.Check();

constexpr ui64 deltaShards = 2;
checks
.PathShardsLimit(deltaShards)
.ShardsLimit(deltaShards);

if (!checks) {
LOG_NOTICE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
"Do not request full stats from datashard"
<< ", datashard: " << datashardId
<< ", reason: " << checks.GetError());
return true;
}
auto path = TPath::Init(pathId, Self);
auto checks = path.Check();
constexpr ui64 deltaShards = 2;
checks
.PathShardsLimit(deltaShards)
.ShardsLimit(deltaShards);
if (!checks) {
LOG_NOTICE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
"Do not request full stats from datashard"
<< ", datashard: " << datashardId
<< ", reason: " << checks.GetError());
return true;
}

if (newStats.HasBorrowedData) {
Expand All @@ -509,6 +505,12 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
return true;
}

if (path.IsLocked()) {
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
"Postpone split tablet " << datashardId << " because it is locked by " << path.LockedBy());
return true;
}

// Request histograms from the datashard
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
"Requesting full tablet stats " << datashardId << " to split it");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ bool TTxPartitionHistogram::Execute(TTransactionContext& txc, const TActorContex
}

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

if (!Self->TabletIdToShardIdx.contains(datashardId)) {
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
Expand All @@ -355,6 +356,12 @@ bool TTxPartitionHistogram::Execute(TTransactionContext& txc, const TActorContex
return true;
}

if (path.IsLocked()) {
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
"TTxPartitionHistogram Skip locked table tablet " << datashardId << " by " << path.LockedBy());
return true;
}

auto shardIdx = Self->TabletIdToShardIdx[datashardId];
const auto forceShardSplitSettings = Self->SplitSettings.GetForceShardSplitSettings();

Expand Down
Loading
Loading