Skip to content

Commit

Permalink
Fixed PR comments ydb-platform#1
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alnen
Alnen committed Jan 31, 2024
1 parent 73b8d3c commit b2e29bf
Show file tree
Hide file tree
Showing 18 changed files with 90 additions and 57 deletions.
8 changes: 7 additions & 1 deletion ydb/core/protos/counters_schemeshard.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,10 @@ enum ESimpleCounters {
COUNTER_IN_FLIGHT_OPS_TxAlterView = 156 [(CounterOpts) = {Name: "InFlightOps/AlterView"}];
COUNTER_IN_FLIGHT_OPS_TxDropView = 157 [(CounterOpts) = {Name: "InFlightOps/DropView"}];

COUNTER_GRAPHSHARD_COUNT = 158 [(CounterOpts) = {Name: "GraphShards"}];
COUNTER_IN_FLIGHT_OPS_TxReplaceExternalTable = 158 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalTable"}];
COUNTER_IN_FLIGHT_OPS_TxReplaceExternalDataSource = 159 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalDataSource"}];

COUNTER_GRAPHSHARD_COUNT = 160 [(CounterOpts) = {Name: "GraphShards"}];
}

enum ECumulativeCounters {
Expand Down Expand Up @@ -315,6 +318,9 @@ enum ECumulativeCounters {
COUNTER_FINISHED_OPS_TxCreateView = 95 [(CounterOpts) = {Name: "FinishedOps/CreateView"}];
COUNTER_FINISHED_OPS_TxDropView = 96 [(CounterOpts) = {Name: "FinishedOps/DropView"}];
COUNTER_FINISHED_OPS_TxAlterView = 97 [(CounterOpts) = {Name: "FinishedOps/AlterView"}];

COUNTER_FINISHED_OPS_TxReplaceExternalTable = 98 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalTable"}];
COUNTER_FINISHED_OPS_TxReplaceExternalDataSource = 99 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalDataSource"}];
}

enum EPercentileCounters {
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/protos/feature_flags.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ message TFeatureFlags {
optional bool EnableServerlessExclusiveDynamicNodes = 113 [default = false];
optional bool EnableAccessServiceBulkAuthorization = 114 [default = false];
optional bool EnableAddColumsWithDefaults = 115 [ default = false];
optional bool EnableReplaceIfExists = 116 [ default = false];
optional bool EnableReplaceIfExistsForExternalEntities = 116 [ default = false];
}
3 changes: 3 additions & 0 deletions ydb/core/protos/flat_scheme_op.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,9 @@ enum EOperationType {
ESchemeOpCreateView = 93;
ESchemeOpAlterView = 94;
ESchemeOpDropView = 95;

ESchemeOpReplaceExternalDataSource = 96;
ESchemeOpReplaceExternalTable = 97;
}

message TApplyIf {
Expand Down
12 changes: 10 additions & 2 deletions ydb/core/tx/schemeshard/schemeshard__operation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1061,14 +1061,18 @@ ISubOperation::TPtr TOperation::RestorePart(TTxState::ETxType txType, TTxState::
return CreateNewExternalTable(NextPartId(), txState);
case TTxState::ETxType::TxDropExternalTable:
return CreateDropExternalTable(NextPartId(), txState);
case TTxState::ETxType::TxReplaceExternalTable:
return CreateReplaceExternalTable(NextPartId(), txState);
case TTxState::ETxType::TxAlterExternalTable:
return CreateAlterExternalTable(NextPartId(), txState);
Y_ABORT("TODO: implement");
case TTxState::ETxType::TxCreateExternalDataSource:
return CreateNewExternalDataSource(NextPartId(), txState);
case TTxState::ETxType::TxDropExternalDataSource:
return CreateDropExternalDataSource(NextPartId(), txState);
case TTxState::ETxType::TxReplaceExternalDataSource:
return CreateReplaceExternalDataSource(NextPartId(), txState);
case TTxState::ETxType::TxAlterExternalDataSource:
return CreateAlterExternalDataSource(NextPartId(), txState);
Y_ABORT("TODO: implement");

// View
case TTxState::ETxType::TxCreateView:
Expand Down Expand Up @@ -1281,6 +1285,8 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op
return CreateDropBlobDepot(NextPartId(), tx);

// ExternalTable
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable:
Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable");
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable:
Y_ABORT("operation is handled before");
case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable:
Expand All @@ -1289,6 +1295,8 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op
Y_ABORT("TODO: implement");

// ExternalDataSource
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource:
Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable");
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource:
Y_ABORT("operation is handled before");
case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource:
Expand Down
8 changes: 4 additions & 4 deletions ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ TVector<ISubOperation::TPtr> CreateNewExternalDataSource(TOperationId id,
Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalDataSource);

LOG_I("CreateNewExternalDataSource, opId " << id << ", feature flag EnableOrReplace "
<< context.SS->EnableReplaceIfExists << ", tx "
<< context.SS->EnableReplaceIfExistsForExternalEntities << ", tx "
<< tx.ShortDebugString());

auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector<ISubOperation::TPtr> {
Expand All @@ -315,8 +315,8 @@ TVector<ISubOperation::TPtr> CreateNewExternalDataSource(TOperationId id,
const auto replaceIfExists = operation.GetReplaceIfExists();
const TString& name = operation.GetName();

if (replaceIfExists && !context.SS->EnableReplaceIfExists) {
return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off");
if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) {
return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off");
}

const TString& parentPathStr = tx.GetWorkingDir();
Expand All @@ -342,7 +342,7 @@ TVector<ISubOperation::TPtr> CreateNewExternalDataSource(TOperationId id,
.IsResolved()
.NotUnderDeleting();
if (isAlreadyExists) {
return {CreateAlterExternalDataSource(id, tx)};
return {CreateReplaceExternalDataSource(id, tx)};
}
}

Expand Down
8 changes: 4 additions & 4 deletions ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ TVector<ISubOperation::TPtr> CreateNewExternalTable(TOperationId id, const TTxTr
Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalTable);

LOG_I("CreateNewExternalTable, opId " << id << ", feature flag EnableOrReplace "
<< context.SS->EnableReplaceIfExists << ", tx "
<< context.SS->EnableReplaceIfExistsForExternalEntities << ", tx "
<< tx.ShortDebugString());

auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector<ISubOperation::TPtr> {
Expand All @@ -408,8 +408,8 @@ TVector<ISubOperation::TPtr> CreateNewExternalTable(TOperationId id, const TTxTr
const auto replaceIfExists = operation.GetReplaceIfExists();
const TString& name = operation.GetName();

if (replaceIfExists && !context.SS->EnableReplaceIfExists) {
return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off");
if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) {
return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off");
}

const TString& parentPathStr = tx.GetWorkingDir();
Expand All @@ -435,7 +435,7 @@ TVector<ISubOperation::TPtr> CreateNewExternalTable(TOperationId id, const TTxTr
.IsResolved()
.NotUnderDeleting();
if (isAlreadyExists) {
return {CreateAlterExternalTable(id, tx)};
return {CreateReplaceExternalTable(id, tx)};
}
}

Expand Down
12 changes: 6 additions & 6 deletions ydb/core/tx/schemeshard/schemeshard__operation_part.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,9 @@ ISubOperation::TPtr CreateUpdateMainTableOnIndexMove(TOperationId id, TTxState::
// Create
TVector<ISubOperation::TPtr> CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context);
ISubOperation::TPtr CreateNewExternalTable(TOperationId id, TTxState::ETxState state);
// Alter
ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state);
// Replace
ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state);
// Drop
ISubOperation::TPtr CreateDropExternalTable(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState state);
Expand All @@ -382,9 +382,9 @@ ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState
// Create
TVector<ISubOperation::TPtr> CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx, TOperationContext& context);
ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, TTxState::ETxState state);
// Alter
ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state);
// Replace
ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state);
// Drop
ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, const TTxTransaction& tx);
ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, TTxState::ETxState state);
Expand Down
26 changes: 13 additions & 13 deletions ..._operation_alter_external_data_source.cpp → ...peration_replace_external_data_source.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class TPropose: public TSubOperationState {

TString DebugHint() const override {
return TStringBuilder()
<< "TAlterExternalDataSource TPropose"
<< "TReplaceExternalDataSource TPropose"
<< ", operationId: " << OperationId;
}

Expand All @@ -34,7 +34,7 @@ class TPropose: public TSubOperationState {

const TTxState* txState = context.SS->FindTx(OperationId);
Y_ABORT_UNLESS(txState);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource);

const auto pathId = txState->TargetPathId;
const auto path = TPath::Init(pathId, context.SS);
Expand All @@ -56,14 +56,14 @@ class TPropose: public TSubOperationState {

const TTxState* txState = context.SS->FindTx(OperationId);
Y_ABORT_UNLESS(txState);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource);

context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0));
return false;
}
};

class TAlterExternalDataSource : public TSubOperation {
class TReplaceExternalDataSource : public TSubOperation {
static TTxState::ETxState NextState() { return TTxState::Propose; }

TTxState::ETxState NextState(TTxState::ETxState state) const override {
Expand Down Expand Up @@ -152,7 +152,7 @@ class TAlterExternalDataSource : public TSubOperation {
void CreateTransaction(const TOperationContext& context,
const TPathId& externalDataSourcePathId) const {
TTxState& txState = context.SS->CreateTx(OperationId,
TTxState::TxAlterExternalDataSource,
TTxState::TxReplaceExternalDataSource,
externalDataSourcePathId);
txState.Shards.clear();
}
Expand Down Expand Up @@ -207,7 +207,7 @@ class TAlterExternalDataSource : public TSubOperation {
Transaction.GetCreateExternalDataSource();
const TString& name = externalDataSourceDescription.GetName();

LOG_N("TAlterExternalDataSource Propose"
LOG_N("TReplaceExternalDataSource Propose"
<< ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name);

auto result = MakeHolder<TProposeResponse>(NKikimrScheme::StatusAccepted,
Expand Down Expand Up @@ -258,13 +258,13 @@ class TAlterExternalDataSource : public TSubOperation {
}

void AbortPropose(TOperationContext& context) override {
LOG_N("TAlterExternalDataSource AbortPropose"
LOG_N("TReplaceExternalDataSource AbortPropose"
<< ": opId# " << OperationId);
Y_ABORT("no AbortPropose for TAlterExternalDataSource");
Y_ABORT("no AbortPropose for TReplaceExternalDataSource");
}

void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override {
LOG_N("TAlterExternalDataSource AbortUnsafe"
LOG_N("TReplaceExternalDataSource AbortUnsafe"
<< ": opId# " << OperationId << ", txId# " << forceDropTxId);
context.OnComplete.DoneOperation(OperationId);
}
Expand All @@ -274,13 +274,13 @@ class TAlterExternalDataSource : public TSubOperation {

namespace NKikimr::NSchemeShard {

ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx) {
return MakeSubOperation<TAlterExternalDataSource>(id, tx);
ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx) {
return MakeSubOperation<TReplaceExternalDataSource>(id, tx);
}

ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state) {
ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state) {
Y_ABORT_UNLESS(state != TTxState::Invalid);
return MakeSubOperation<TAlterExternalDataSource>(id, state);
return MakeSubOperation<TReplaceExternalDataSource>(id, state);
}

}
26 changes: 13 additions & 13 deletions ...shard__operation_alter_external_table.cpp → ...ard__operation_replace_external_table.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class TPropose: public TSubOperationState {

TString DebugHint() const override {
return TStringBuilder()
<< "TAlterExternalTable TPropose"
<< "TReplaceExternalTable TPropose"
<< ", operationId: " << OperationId;
}

Expand Down Expand Up @@ -59,7 +59,7 @@ class TPropose: public TSubOperationState {

const TTxState* txState = context.SS->FindTx(OperationId);
Y_ABORT_UNLESS(txState);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable);

const auto pathId = txState->TargetPathId;
const auto dataSourcePathId = txState->SourcePathId;
Expand Down Expand Up @@ -88,15 +88,15 @@ class TPropose: public TSubOperationState {

const TTxState* txState = context.SS->FindTx(OperationId);
Y_ABORT_UNLESS(txState);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable);
Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable);

context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0));
return false;
}
};


class TAltertExternalTable: public TSubOperation {
class TReplacetExternalTable: public TSubOperation {
private:
bool IsSameDataSource = true;
TPathId OldDataSourcePathId = InvalidPathId;
Expand Down Expand Up @@ -220,7 +220,7 @@ class TAltertExternalTable: public TSubOperation {
const TPathId& externalTablePathId,
const TPathId& externalDataSourcePathId) const {
TTxState& txState = context.SS->CreateTx(OperationId,
TTxState::TxAlterExternalTable,
TTxState::TxReplaceExternalTable,
externalTablePathId,
externalDataSourcePathId);
txState.Shards.clear();
Expand Down Expand Up @@ -301,7 +301,7 @@ class TAltertExternalTable: public TSubOperation {
const auto& externalTableDescription = Transaction.GetCreateExternalTable();
const TString& name = externalTableDescription.GetName();

LOG_N("TAlterExternalTable Propose"
LOG_N("TReplaceExternalTable Propose"
<< ": opId# " << OperationId
<< ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists:" << externalTableDescription.GetReplaceIfExists());

Expand Down Expand Up @@ -392,13 +392,13 @@ class TAltertExternalTable: public TSubOperation {
}

void AbortPropose(TOperationContext& context) override {
LOG_N("TAlterExternalTable AbortPropose"
LOG_N("TReplaceExternalTable AbortPropose"
<< ": opId# " << OperationId);
Y_ABORT("no AbortPropose for TAlterExternalTable");
Y_ABORT("no AbortPropose for TReplaceExternalTable");
}

void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override {
LOG_N("TAlterExternalTable AbortUnsafe"
LOG_N("TReplaceExternalTable AbortUnsafe"
<< ": opId# " << OperationId
<< ", txId# " << forceDropTxId);
context.OnComplete.DoneOperation(OperationId);
Expand All @@ -409,13 +409,13 @@ class TAltertExternalTable: public TSubOperation {

namespace NKikimr::NSchemeShard {

ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx) {
return MakeSubOperation<TAltertExternalTable>(std::move(id), tx);
ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx) {
return MakeSubOperation<TReplacetExternalTable>(std::move(id), tx);
}

ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state) {
ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state) {
Y_ABORT_UNLESS(state != TTxState::Invalid);
return MakeSubOperation<TAltertExternalTable>(std::move(id), state);
return MakeSubOperation<TReplacetExternalTable>(std::move(id), state);
}

}
6 changes: 5 additions & 1 deletion ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,14 @@ TString DefineUserOperationName(const NKikimrSchemeOp::TModifyScheme& tx) {
return "ALTER BLOB DEPOT";
case NKikimrSchemeOp::EOperationType::ESchemeOpDropBlobDepot:
return "DROP BLOB DEPOT";
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable:
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable:
return "CREATE EXTERNAL TABLE";
case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable:
return "DROP EXTERNAL TABLE";
case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable:
return "ALTER EXTERNAL TABLE";
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource:
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource:
return "CREATE EXTERNAL DATA SOURCE";
case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource:
Expand Down Expand Up @@ -487,6 +489,7 @@ TVector<TString> ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx)
result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetSrcPath()}));
result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetDstPath()}));
break;
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable:
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable:
result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalTable().GetName()}));
break;
Expand All @@ -496,6 +499,7 @@ TVector<TString> ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx)
case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable:
// TODO: unimplemented
break;
case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource:
case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource:
result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalDataSource().GetName()}));
break;
Expand Down Expand Up @@ -636,7 +640,7 @@ TAuditLogFragment MakeAuditLogFragment(const NKikimrSchemeOp::TModifyScheme& tx)
auto [aclAdd, aclRemove] = ExtractACLChange(tx);
auto [userAttrsAdd, userAttrsRemove] = ExtractUserAttrChange(tx);
auto [loginUser, loginGroup, loginMember] = ExtractLoginChange(tx);

return {
.Operation = DefineUserOperationName(tx),
.Paths = ExtractChangingPaths(tx),
Expand Down
Loading

0 comments on commit b2e29bf

Please sign in to comment.