Skip to content

Commit 3536ce2

Browse files
swalrus1swalrus1
committed
fix risk: client with new proto, server with old proto -> server interprets expireInSeconds as 0
1 parent 8e107d4 commit 3536ce2

File tree

5 files changed

+108
-108
lines changed

5 files changed

+108
-108
lines changed

‎ydb/core/ydb_convert/table_description.cpp‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -507,23 +507,6 @@ Ydb::Type* AddColumn<NKikimrSchemeOp::TColumnDescription>(Ydb::Table::ColumnMeta
507507

508508
template <typename TYdbProto, typename TTtl>
509509
static void AddTtl(TYdbProto& out, const TTtl& inTTL) {
510-
static const auto& fillCommonFields = []<class TModeSettings>(TModeSettings& out, const TTtl& in) {
511-
out.set_column_name(in.GetColumnName());
512-
if (in.TiersSize()) {
513-
std::optional<ui32> expireInSeconds;
514-
for (const auto& inTier : in.GetTiers()) {
515-
if (inTier.HasDelete()) {
516-
expireInSeconds = inTier.GetEvictAfterSeconds();
517-
break;
518-
}
519-
}
520-
out.set_expire_after_seconds(expireInSeconds.value_or(std::numeric_limits<uint32_t>::max()));
521-
} else {
522-
// legacy schema
523-
out.set_expire_after_seconds(in.GetExpireAfterSeconds());
524-
}
525-
};
526-
527510
for (const auto& inTier : inTTL.GetTiers()) {
528511
auto* outTier = out.mutable_ttl_settings()->add_tiers();
529512
outTier->set_evict_after_seconds(inTier.GetEvictAfterSeconds());
@@ -542,7 +525,7 @@ static void AddTtl(TYdbProto& out, const TTtl& inTTL) {
542525
switch (inTTL.GetColumnUnit()) {
543526
case NKikimrSchemeOp::TTTLSettings::UNIT_AUTO: {
544527
auto& outTTL = *out.mutable_ttl_settings()->mutable_date_type_column();
545-
fillCommonFields(outTTL, inTTL);
528+
outTTL.set_column_name(inTTL.GetColumnName());
546529
break;
547530
}
548531

@@ -551,7 +534,7 @@ static void AddTtl(TYdbProto& out, const TTtl& inTTL) {
551534
case NKikimrSchemeOp::TTTLSettings::UNIT_MICROSECONDS:
552535
case NKikimrSchemeOp::TTTLSettings::UNIT_NANOSECONDS: {
553536
auto& outTTL = *out.mutable_ttl_settings()->mutable_value_since_unix_epoch();
554-
fillCommonFields(outTTL, inTTL);
537+
outTTL.set_column_name(inTTL.GetColumnName());
555538
outTTL.set_column_unit(static_cast<Ydb::Table::ValueSinceUnixEpochModeSettings::Unit>(inTTL.GetColumnUnit()));
556539
break;
557540
}

‎ydb/core/ydb_convert/table_settings.h‎

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,42 @@ bool FillTtlSettings(TTtlSettingsEnabled& out, const Ydb::Table::TtlSettings& in
2828
return false;
2929
};
3030

31-
static const auto& fillCommonFields = []<class TModeSettings>(TTtlSettingsEnabled& out, const TModeSettings& in, std::optional<ui32> expireAfterSeconds) {
31+
static const auto& fillModeSettings = []<class TModeSettings>(TTtlSettingsEnabled& out, const TModeSettings& in) {
3232
out.SetColumnName(in.column_name());
33-
out.SetExpireAfterSeconds(expireAfterSeconds.value_or(in.expire_after_seconds()));
3433
};
3534

36-
std::optional<ui32> expireAfterSeconds;
35+
static const auto& fillDeleteTier = []<class TModeSettings>(TTtlSettingsEnabled& out, const TModeSettings& in) {
36+
auto* deleteTier = out.AddTiers();
37+
deleteTier->SetEvictAfterSeconds(in.expire_after_seconds());
38+
deleteTier->MutableDelete();
39+
};
40+
41+
static const auto& fillColumnUnit = []<class TModeSettings> (TTtlSettingsEnabled& out, const TModeSettings& in) -> bool {
42+
#define CASE_UNIT(type) \
43+
case Ydb::Table::ValueSinceUnixEpochModeSettings::type: \
44+
out.SetColumnUnit(NKikimrSchemeOp::TTTLSettings::type); \
45+
break
46+
47+
switch (in.column_unit()) {
48+
CASE_UNIT(UNIT_SECONDS);
49+
CASE_UNIT(UNIT_MILLISECONDS);
50+
CASE_UNIT(UNIT_MICROSECONDS);
51+
CASE_UNIT(UNIT_NANOSECONDS);
52+
default:
53+
return unsupported(TStringBuilder() << "Unsupported unit: "
54+
<< static_cast<ui32>(in.value_since_unix_epoch().column_unit()));
55+
}
56+
57+
#undef CASE_UNIT
58+
};
59+
3760
if (in.tiers_size()) {
3861
for (const auto& inTier : in.tiers()) {
3962
auto* outTier = out.AddTiers();
4063
outTier->SetEvictAfterSeconds(inTier.evict_after_seconds());
4164
switch (inTier.action_case()) {
4265
case Ydb::Table::TtlTier::kDelete:
4366
outTier->MutableDelete();
44-
expireAfterSeconds = inTier.evict_after_seconds();
4567
break;
4668
case Ydb::Table::TtlTier::kEvictToExternalStorage:
4769
outTier->MutableEvictToExternalStorage()->SetStorageName(inTier.evict_to_external_storage().storage_name());
@@ -50,41 +72,45 @@ bool FillTtlSettings(TTtlSettingsEnabled& out, const Ydb::Table::TtlSettings& in
5072
break;
5173
}
5274
}
53-
if (!expireAfterSeconds) {
54-
expireAfterSeconds = std::numeric_limits<uint32_t>::max();
55-
}
5675
}
5776

5877
switch (in.mode_case()) {
59-
case Ydb::Table::TtlSettings::kDateTypeColumn:
60-
fillCommonFields(out, in.date_type_column(), expireAfterSeconds);
78+
case Ydb::Table::TtlSettings::kDeprecatedDateTypeColumn:
79+
fillModeSettings(out, in.deprecated_date_type_column());
80+
fillDeleteTier(out, in.deprecated_date_type_column());
6181
break;
6282

63-
case Ydb::Table::TtlSettings::kValueSinceUnixEpoch:
64-
fillCommonFields(out, in.value_since_unix_epoch(), expireAfterSeconds);
83+
case Ydb::Table::TtlSettings::kDeprecatedValueSinceUnixEpoch:
84+
fillModeSettings(out, in.deprecated_value_since_unix_epoch());
85+
fillDeleteTier(out, in.deprecated_date_type_column());
86+
if (!fillColumnUnit(out, in.value_since_unix_epoch())) {
87+
return false;
88+
}
89+
break;
6590

66-
#define CASE_UNIT(type) \
67-
case Ydb::Table::ValueSinceUnixEpochModeSettings::type: \
68-
out.SetColumnUnit(NKikimrSchemeOp::TTTLSettings::type); \
69-
break
91+
case Ydb::Table::TtlSettings::kDateTypeColumn:
92+
fillModeSettings(out, in.date_type_column());
93+
break;
7094

71-
switch (in.value_since_unix_epoch().column_unit()) {
72-
CASE_UNIT(UNIT_SECONDS);
73-
CASE_UNIT(UNIT_MILLISECONDS);
74-
CASE_UNIT(UNIT_MICROSECONDS);
75-
CASE_UNIT(UNIT_NANOSECONDS);
76-
default:
77-
return unsupported(TStringBuilder() << "Unsupported unit: "
78-
<< static_cast<ui32>(in.value_since_unix_epoch().column_unit()));
95+
case Ydb::Table::TtlSettings::kValueSinceUnixEpoch:
96+
fillModeSettings(out, in.value_since_unix_epoch());
97+
if (!fillColumnUnit(out, in.value_since_unix_epoch())) {
98+
return false;
7999
}
80-
81-
#undef CASE_UNIT
82100
break;
83101

84-
default:
102+
case Ydb::Table::TtlSettings::MODE_NOT_SET:
85103
return unsupported("Unsupported ttl settings");
86104
}
87105

106+
std::optional<ui32> expireInSeconds = 0;
107+
for (const auto& tier : out.GetTiers()) {
108+
if (tier.HasDelete()) {
109+
expireInSeconds = tier.GetEvictInSeconds();
110+
}
111+
}
112+
out.SetExpireInSeconds(expireInSeconds.value_or(std::numeric_limits<uint32_t>::max()));
113+
88114
if constexpr (std::is_same_v<TTtlSettingsEnabled, NKikimrSchemeOp::TTTLSettings::TEnabled>) {
89115
if (in.run_interval_seconds()) {
90116
out.MutableSysSettings()->SetRunInterval(TDuration::Seconds(in.run_interval_seconds()).GetValue());

‎ydb/public/api/protos/ydb_table.proto‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -453,17 +453,13 @@ message TtlTier {
453453
}
454454

455455
message DateTypeColumnModeSettings {
456-
// The row will be belong to a tier at the moment of time, when the value
456+
// The row will be assigned a tier at the moment of time, when the value
457457
// stored in <column_name> is less than or equal to the current time (in epoch
458-
// time format), and <evict_after_seconds> has passed since that moment;
459-
// i.e. the eviction threshold is the value of <column_name> plus <evict_after_seconds>.
458+
// time format), and <tier.evict_after_seconds> has passed since that moment;
459+
// i.e. the eviction threshold is the value of <column_name> plus <tier.evict_after_seconds>.
460460

461461
// The column type must be a date type
462462
string column_name = 1;
463-
464-
// Deprecated, replaced by TtlSettings.tiers. If TtlSettings.tiers is not empty,
465-
// expire_after_seconds is ignored
466-
uint32 expire_after_seconds = 2;
467463
}
468464

469465
message ValueSinceUnixEpochModeSettings {
@@ -486,16 +482,27 @@ message ValueSinceUnixEpochModeSettings {
486482

487483
// Interpretation of the value stored in <column_name>
488484
Unit column_unit = 2;
485+
}
486+
487+
message DeprecatedDateTypeColumnModeSettings {
488+
// Deprecated TTL mode for internal use only
489+
string column_name = 1;
490+
uint32 expire_after_seconds = 2;
491+
}
489492

490-
// Deprecated, replaced by TtlSettings.tiers. If TtlSettings.tiers is not empty,
491-
// expire_after_seconds is ignored
493+
message DeprecatedValueSinceUnixEpochModeSettings {
494+
// Deprecated TTL mode for internal use only
495+
string column_name = 1;
496+
ValueSinceUnixEpochModeSettings.Unit column_unit = 2;
492497
uint32 expire_after_seconds = 3;
493498
}
494499

495500
message TtlSettings {
496501
oneof mode {
497-
DateTypeColumnModeSettings date_type_column = 1;
498-
ValueSinceUnixEpochModeSettings value_since_unix_epoch = 2;
502+
DeprecatedDateTypeColumnModeSettings deprecated_date_type_column = 1 [deprecated = true];
503+
DeprecatedValueSinceUnixEpochModeSettings deprecated_value_since_unix_epoch = 2 [deprecated = true];
504+
DateTypeColumnModeSettings date_type_column = 4;
505+
ValueSinceUnixEpochModeSettings value_since_unix_epoch = 5;
499506
}
500507

501508
// There is no guarantee that expired row will be deleted immediately upon
@@ -512,7 +519,7 @@ message TtlSettings {
512519
// BRO will not be started more often, but may be started less often.
513520
uint32 run_interval_seconds = 3;
514521

515-
repeated TtlTier tiers = 4;
522+
repeated TtlTier tiers = 6;
516523
}
517524

518525
message StorageSettings {

‎ydb/public/sdk/cpp/client/ydb_table/table.cpp‎

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -327,23 +327,8 @@ class TTableDescription::TImpl {
327327
}
328328

329329
// ttl settings
330-
switch (proto.ttl_settings().mode_case()) {
331-
case Ydb::Table::TtlSettings::kDateTypeColumn:
332-
TtlSettings_ = TTtlSettings(
333-
proto.ttl_settings().date_type_column(),
334-
proto.ttl_settings().run_interval_seconds()
335-
);
336-
break;
337-
338-
case Ydb::Table::TtlSettings::kValueSinceUnixEpoch:
339-
TtlSettings_ = TTtlSettings(
340-
proto.ttl_settings().value_since_unix_epoch(),
341-
proto.ttl_settings().run_interval_seconds()
342-
);
343-
break;
344-
345-
default:
346-
break;
330+
if (auto ttlSettings = TTtlSettings::DeserializeFromProto(proto.ttl_settings())) {
331+
TtlSettings_ = std::move(*ttlSettings);
347332
}
348333

349334
if (proto.store_type()) {
@@ -2938,12 +2923,12 @@ TVector<TTtlTierSettings> DeserializeTiers(const NProtoBuf::RepeatedPtrField<Ydb
29382923
} // namespace
29392924

29402925
TTtlTierSettings::TTtlTierSettings(TDuration evictionDelay, const TAction& action)
2941-
: EvictionDelay_(evictionDelay)
2926+
: EvictAfter_(evictionDelay)
29422927
, Action_(action) {
29432928
}
29442929

29452930
TTtlTierSettings::TTtlTierSettings(const Ydb::Table::TtlTier& tier)
2946-
: EvictionDelay_(TDuration::Seconds(tier.evict_after_seconds())) {
2931+
: EvictAfter_(TDuration::Seconds(tier.evict_after_seconds())) {
29472932
switch (tier.action_case()) {
29482933
case Ydb::Table::TtlTier::kDelete:
29492934
Action_ = TTtlDeleteAction();
@@ -2957,7 +2942,7 @@ TTtlTierSettings::TTtlTierSettings(const Ydb::Table::TtlTier& tier)
29572942
}
29582943

29592944
void TTtlTierSettings::SerializeTo(Ydb::Table::TtlTier& proto) const {
2960-
proto.set_evict_after_seconds(EvictionDelay_.Seconds());
2945+
proto.set_evict_after_seconds(EvictAfter_.Seconds());
29612946

29622947
std::visit(TOverloaded{
29632948
[&proto](const TTtlDeleteAction&) { proto.mutable_delete_(); },
@@ -2969,8 +2954,8 @@ void TTtlTierSettings::SerializeTo(Ydb::Table::TtlTier& proto) const {
29692954
Action_);
29702955
}
29712956

2972-
TDuration TTtlTierSettings::GetEvictionDelay() const {
2973-
return EvictionDelay_;
2957+
TDuration TTtlTierSettings::GetEvictAfter() const {
2958+
return EvictAfter_;
29742959
}
29752960

29762961
const TTtlTierSettings::TAction& TTtlTierSettings::GetAction() const {
@@ -2984,7 +2969,6 @@ TDateTypeColumnModeSettings::TDateTypeColumnModeSettings(const TString& columnNa
29842969

29852970
void TDateTypeColumnModeSettings::SerializeTo(Ydb::Table::DateTypeColumnModeSettings& proto) const {
29862971
proto.set_column_name(ColumnName_);
2987-
proto.set_expire_after_seconds(Min((TDuration::TValue)std::numeric_limits<uint32_t>::max(), DeprecatedExpireAfter_.Seconds()));
29882972
}
29892973

29902974
const TString& TDateTypeColumnModeSettings::GetColumnName() const {
@@ -3004,7 +2988,6 @@ TValueSinceUnixEpochModeSettings::TValueSinceUnixEpochModeSettings(const TString
30042988
void TValueSinceUnixEpochModeSettings::SerializeTo(Ydb::Table::ValueSinceUnixEpochModeSettings& proto) const {
30052989
proto.set_column_name(ColumnName_);
30062990
proto.set_column_unit(TProtoAccessor::GetProto(ColumnUnit_));
3007-
proto.set_expire_after_seconds(Min((TDuration::TValue)std::numeric_limits<uint32_t>::max(), DeprecatedExpireAfter_.Seconds()));
30082991
}
30092992

30102993
const TString& TValueSinceUnixEpochModeSettings::GetColumnName() const {
@@ -3060,14 +3043,14 @@ TValueSinceUnixEpochModeSettings::EUnit TValueSinceUnixEpochModeSettings::UnitFr
30603043
}
30613044

30623045
TTtlSettings::TTtlSettings(const TString& columnName, const TVector<TTtlTierSettings>& tiers)
3063-
: Mode_(TDateTypeColumnModeSettings(columnName, GetExpirationDelay(tiers).value_or(TDuration::Max())))
3046+
: Mode_(TDateTypeColumnModeSettings(columnName, GetExpireAfterFrom(tiers).value_or(TDuration::Max())))
30643047
{}
30653048

30663049
TTtlSettings::TTtlSettings(const TString& columnName, const TDuration& expireAfter)
30673050
: TTtlSettings(columnName, {TTtlTierSettings(expireAfter, TTtlDeleteAction())})
30683051
{}
30693052

3070-
TTtlSettings::TTtlSettings(const Ydb::Table::DateTypeColumnModeSettings& mode, ui32 runIntervalSeconds)
3053+
TTtlSettings::TTtlSettings(const Ydb::Table::DeprecatedDateTypeColumnModeSettings& mode, ui32 runIntervalSeconds)
30713054
: TTtlSettings(mode.column_name(), TDuration::Seconds(mode.expire_after_seconds())) {
30723055
RunInterval_ = TDuration::Seconds(runIntervalSeconds);
30733056
}
@@ -3077,14 +3060,14 @@ const TDateTypeColumnModeSettings& TTtlSettings::GetDateTypeColumn() const {
30773060
}
30783061

30793062
TTtlSettings::TTtlSettings(const TString& columnName, EUnit columnUnit, const TVector<TTtlTierSettings>& tiers)
3080-
: Mode_(TValueSinceUnixEpochModeSettings(columnName, columnUnit, GetExpirationDelay(tiers).value_or(TDuration::Max())))
3063+
: Mode_(TValueSinceUnixEpochModeSettings(columnName, columnUnit, GetExpireAfterFrom(tiers).value_or(TDuration::Max())))
30813064
{}
30823065

30833066
TTtlSettings::TTtlSettings(const TString& columnName, EUnit columnUnit, const TDuration& expireAfter)
30843067
: TTtlSettings(columnName, columnUnit, {TTtlTierSettings(expireAfter, TTtlDeleteAction())})
30853068
{}
30863069

3087-
TTtlSettings::TTtlSettings(const Ydb::Table::ValueSinceUnixEpochModeSettings& mode, ui32 runIntervalSeconds)
3070+
TTtlSettings::TTtlSettings(const Ydb::Table::DeprecatedValueSinceUnixEpochModeSettings& mode, ui32 runIntervalSeconds)
30883071
: TTtlSettings(mode.column_name(), TProtoAccessor::FromProto(mode.column_unit()), TDuration::Seconds(mode.expire_after_seconds())) {
30893072
RunInterval_ = TDuration::Seconds(runIntervalSeconds);
30903073
}
@@ -3094,28 +3077,21 @@ const TValueSinceUnixEpochModeSettings& TTtlSettings::GetValueSinceUnixEpoch() c
30943077
}
30953078

30963079
std::optional<TTtlSettings> TTtlSettings::DeserializeFromProto(const Ydb::Table::TtlSettings& proto) {
3097-
if (proto.tiers_size() == 0) {
3098-
// legacy schema
3099-
switch(proto.mode_case()) {
3100-
case Ydb::Table::TtlSettings::kDateTypeColumn:
3101-
return TTtlSettings(proto.date_type_column(), proto.run_interval_seconds());
3102-
case Ydb::Table::TtlSettings::kValueSinceUnixEpoch:
3103-
return TTtlSettings(proto.value_since_unix_epoch(), proto.run_interval_seconds());
3104-
case Ydb::Table::TtlSettings::MODE_NOT_SET:
3105-
return std::nullopt;
3106-
}
3107-
}
3108-
31093080
auto tiers = DeserializeTiers(proto.tiers());
3110-
const TDuration legacyExpireAfter = GetExpirationDelay(tiers).value_or(TDuration::Max());
3081+
const TDuration legacyExpireAfter = GetExpireAfterFrom(tiers).value_or(TDuration::Max());
31113082

31123083
switch(proto.mode_case()) {
3084+
case Ydb::Table::TtlSettings::kDeprecatedDateTypeColumn:
3085+
return TTtlSettings(proto.deprecated_date_type_column(), proto.run_interval_seconds());
3086+
case Ydb::Table::TtlSettings::kDeprecatedValueSinceUnixEpoch:
3087+
return TTtlSettings(proto.deprecated_value_since_unix_epoch(), proto.run_interval_seconds());
31133088
case Ydb::Table::TtlSettings::kDateTypeColumn:
31143089
return TTtlSettings(TDateTypeColumnModeSettings(proto.date_type_column().column_name(), legacyExpireAfter), proto.run_interval_seconds());
31153090
case Ydb::Table::TtlSettings::kValueSinceUnixEpoch:
31163091
return TTtlSettings(TValueSinceUnixEpochModeSettings(proto.date_type_column().column_name(), TProtoAccessor::FromProto(proto.value_since_unix_epoch().column_unit()), legacyExpireAfter), proto.run_interval_seconds());
31173092
case Ydb::Table::TtlSettings::MODE_NOT_SET:
31183093
return std::nullopt;
3094+
break;
31193095
}
31203096
}
31213097

@@ -3151,10 +3127,14 @@ const TVector<TTtlTierSettings>& TTtlSettings::GetTiers() const {
31513127
return Tiers_;
31523128
}
31533129

3154-
std::optional<TDuration> TTtlSettings::GetExpirationDelay(const TVector<TTtlTierSettings>& tiers) {
3130+
std::optional<TDuration> TTtlSettings::GetExpireAfter() const {
3131+
return GetExpireAfterFrom(Tiers_);
3132+
}
3133+
3134+
std::optional<TDuration> TTtlSettings::GetExpireAfterFrom(const TVector<TTtlTierSettings>& tiers) {
31553135
for (const auto& tier : tiers) {
31563136
if (std::holds_alternative<TTtlDeleteAction>(tier.GetAction())) {
3157-
return tier.GetEvictionDelay();
3137+
return tier.GetEvictAfter();
31583138
}
31593139
}
31603140
return std::nullopt;

0 commit comments

Comments
 (0)