aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorserg-belyakov <serg-belyakov@yandex-team.com>2023-08-22 15:03:44 +0300
committerserg-belyakov <serg-belyakov@yandex-team.com>2023-08-22 15:37:44 +0300
commit037360c096adbb72d4a50fb5de0db80368da03be (patch)
treea9cf0196e866746d0c08edd2108f4468757572ad
parent39b001b8899b48037e0d669b1bf6dad6c4ad8e48 (diff)
downloadydb-037360c096adbb72d4a50fb5de0db80368da03be.tar.gz
Always use common +-1 major rule, KIKIMR-15989
Always use common compatibility rule
-rw-r--r--ydb/core/driver_lib/version/ut/version_ut.cpp39
-rw-r--r--ydb/core/driver_lib/version/version.cpp123
2 files changed, 59 insertions, 103 deletions
diff --git a/ydb/core/driver_lib/version/ut/version_ut.cpp b/ydb/core/driver_lib/version/ut/version_ut.cpp
index a6a79078f74..8dbeecbf569 100644
--- a/ydb/core/driver_lib/version/ut/version_ut.cpp
+++ b/ydb/core/driver_lib/version/ut/version_ut.cpp
@@ -14,6 +14,7 @@ Y_UNIT_TEST_SUITE(VersionParser) {
}
}
+using TComponentId = NKikimrConfig::TCompatibilityRule::EComponentId;
using EComponentId = NKikimrConfig::TCompatibilityRule;
using TOldFormat = NActors::TInterconnectProxyCommon::TVersionInfo;
using TVersion = TCompatibilityInfo::TProtoConstructor::TVersion;
@@ -23,13 +24,14 @@ using TStoredCompatibilityInfo = TCompatibilityInfo::TProtoConstructor::TStoredC
Y_UNIT_TEST_SUITE(YdbVersion) {
- void Test(TCurrentCompatibilityInfo current, TCurrentCompatibilityInfo store, bool expected) {
+ void Test(TCurrentCompatibilityInfo current, TCurrentCompatibilityInfo store, bool expected,
+ TComponentId componentId = EComponentId::Test1) {
TString errorReason;
auto currentPB = current.ToPB();
auto storePB = store.ToPB();
- auto storedPB = CompatibilityInfo.MakeStored(EComponentId::Test1, &storePB);
+ auto storedPB = CompatibilityInfo.MakeStored(componentId, &storePB);
UNIT_ASSERT_EQUAL_C(CompatibilityInfo.CheckCompatibility(&currentPB, &storedPB,
- EComponentId::Test1, errorReason), expected, errorReason);
+ componentId, errorReason), expected, errorReason);
}
Y_UNIT_TEST(DefaultSameVersion) {
@@ -609,6 +611,7 @@ Y_UNIT_TEST_SUITE(YdbVersion) {
.Application = "nbs",
.LowerLimit = TVersion{ .Year = 23, .Major = 3 },
.UpperLimit = TVersion{ .Year = 24, .Major = 2 },
+ .ComponentId = EComponentId::Interconnect,
},
},
.StoresReadableBy = {
@@ -616,10 +619,12 @@ Y_UNIT_TEST_SUITE(YdbVersion) {
.Application = "nbs",
.LowerLimit = TVersion{ .Year = 23, .Major = 3 },
.UpperLimit = TVersion{ .Year = 24, .Major = 2 },
+ .ComponentId = EComponentId::Interconnect,
},
}
},
- true
+ true,
+ EComponentId::Interconnect
);
}
@@ -665,32 +670,6 @@ Y_UNIT_TEST_SUITE(YdbVersion) {
);
}
- Y_UNIT_TEST(RestrictedCompatibilitySameApplication) {
- Test(
- TCurrentCompatibilityInfo{
- .Application = "ydb",
- .Version = TVersion{ .Year = 24, .Major = 3, .Minor = 1, .Hotfix = 0 },
- .CanLoadFrom = {
- TCompatibilityRule{
- .LowerLimit = TVersion{ .Year = 24, .Major = 2, .Minor = 4},
- .UpperLimit = TVersion{ .Year = 24, .Major = 3 },
- },
- },
- .StoresReadableBy = {
- TCompatibilityRule{
- .LowerLimit = TVersion{ .Year = 24, .Major = 2, .Minor = 4},
- .UpperLimit = TVersion{ .Year = 24, .Major = 3 },
- },
- }
- },
- TCurrentCompatibilityInfo{
- .Application = "ydb",
- .Version = TVersion{ .Year = 24, .Major = 2, .Minor = 1, .Hotfix = 0 },
- },
- false
- );
- }
-
Y_UNIT_TEST(CompatibleWithSelf) {
auto stored = CompatibilityInfo.MakeStored(EComponentId::Test1);
TString errorReason;
diff --git a/ydb/core/driver_lib/version/version.cpp b/ydb/core/driver_lib/version/version.cpp
index 439e6c96a08..31d39326ff0 100644
--- a/ydb/core/driver_lib/version/version.cpp
+++ b/ydb/core/driver_lib/version/version.cpp
@@ -95,12 +95,12 @@ TString PrintStoredAndCurrent(const TStored* stored, const TCurrent* current) {
"Current CompatibilityInfo# { " << currentStr << " } ";
}
-TString PrintStoredAndCurrent(const TOldFormat& stored, const TCurrent* current) {
+TString PrintStoredAndCurrent(const TOldFormat& peer, const TCurrent* current) {
TStringStream str;
- str << "Stored CompatibilityInfo# { ";
- str << "Tag# " << stored.Tag;
+ str << "Peer CompatibilityInfo# { ";
+ str << "Tag# " << peer.Tag;
str << "AcceptedTag# { ";
- for (const TString& tag : stored.AcceptedTags) {
+ for (const TString& tag : peer.AcceptedTags) {
str << tag << " ";
}
str << " } } ";
@@ -119,8 +119,7 @@ TStored TCompatibilityInfo::MakeStored(TComponentId componentId, const TCurrent*
stored.MutableVersion()->CopyFrom(current->GetVersion());
}
- for (ui32 i = 0; i < current->StoresReadableBySize(); i++) {
- auto rule = current->GetStoresReadableBy(i);
+ for (const auto& rule : current->GetStoresReadableBy()) {
const auto ruleComponentId = TComponentId(rule.GetComponentId());
if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) {
auto *newRule = stored.AddReadableBy();
@@ -132,7 +131,6 @@ TStored TCompatibilityInfo::MakeStored(TComponentId componentId, const TCurrent*
newRule->SetForbidden(rule.GetForbidden());
}
}
-
return stored;
}
@@ -279,18 +277,12 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor
const auto* storedVersion = stored->HasVersion() ? &stored->GetVersion() : nullptr;
bool permitted = false;
- bool useDefault = true;
- for (ui32 i = 0; i < current->CanLoadFromSize(); ++i) {
- const auto rule = current->GetCanLoadFrom(i);
+ for (const auto& rule : current->GetCanLoadFrom()) {
const auto ruleComponentId = TComponentId(rule.GetComponentId());
if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) {
- bool isForbidding = rule.HasForbidden() && rule.GetForbidden();
- if ((!rule.HasApplication() || rule.GetApplication() == storedApplication) && !isForbidding) {
- useDefault = false;
- }
if (CheckRule(storedApplication, storedVersion, rule)) {
- if (isForbidding) {
+ if (rule.HasForbidden() && rule.GetForbidden()) {
errorReason = "Stored version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current);
return false;
} else {
@@ -300,12 +292,10 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor
}
}
- for (ui32 i = 0; i < stored->ReadableBySize(); ++i) {
- const auto rule = stored->GetReadableBy(i);
+ for (const auto& rule : stored->GetReadableBy()) {
const auto ruleComponentId = TComponentId(rule.GetComponentId());
if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) {
if (CheckRule(currentApplication, currentVersion, rule)) {
- useDefault = false;
if (rule.HasForbidden() && rule.GetForbidden()) {
errorReason = "Current version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current);
return false;
@@ -319,16 +309,13 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TStor
if (permitted) {
return true;
} else {
- if (useDefault) {
- if (CheckDefaultRules(currentApplication, currentVersion, storedApplication, storedVersion)) {
- return true;
- } else {
- errorReason = "Versions are not compatible by default rules, " + PrintStoredAndCurrent(stored, current);
- return false;
- }
+ if (CheckDefaultRules(currentApplication, currentVersion, storedApplication, storedVersion)) {
+ return true;
+ } else {
+ errorReason = "Versions are not compatible neither by common rule nor by provided rule sets, "
+ + PrintStoredAndCurrent(stored, current);
+ return false;
}
- errorReason = "Versions are not compatible by given rule sets, " + PrintStoredAndCurrent(stored, current);
- return false;
}
}
@@ -552,6 +539,7 @@ void CheckVersionTag() {
}
bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldFormat& stored, TComponentId componentId, TString& errorReason) const {
+ // stored version is peer version in terms of Interconnect
Y_VERIFY(current);
std::optional<TString> storedApplication;
@@ -566,18 +554,12 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldF
}
bool permitted = false;
- bool useDefault = true;
- for (ui32 i = 0; i < current->CanLoadFromSize(); ++i) {
- const auto rule = current->GetCanLoadFrom(i);
+ for (const auto& rule : current->GetCanLoadFrom()) {
const auto ruleComponentId = TComponentId(rule.GetComponentId());
if (!rule.HasComponentId() || ruleComponentId == componentId || ruleComponentId == EComponentId::Any) {
- bool isForbidding = rule.HasForbidden() && rule.GetForbidden();
- if (!rule.HasApplication() && !isForbidding) {
- useDefault = false;
- }
if (CheckRule(storedApplication, &*storedVersion, rule)) {
- if (isForbidding) {
+ if (rule.HasForbidden() && rule.GetForbidden()) {
errorReason = "Stored version is explicitly prohibited, " + PrintStoredAndCurrent(stored, current);
return false;
} else {
@@ -591,60 +573,55 @@ bool TCompatibilityInfo::CheckCompatibility(const TCurrent* current, const TOldF
return true;
}
- const auto* currentVersion = current->HasVersion() ? &current->GetVersion() : nullptr;
for (const auto& tag : stored.AcceptedTags) {
auto version = ParseVersionFromTag(tag);
- if (storedVersion && currentVersion) {
- if (version->GetYear() == currentVersion->GetYear() &&
- version->GetMajor() == currentVersion->GetMajor() &&
- version->GetMinor() == currentVersion->GetMinor() &&
- version->GetHotfix() == currentVersion->GetHotfix()) {
+ if (version && current->HasVersion()) {
+ if (CompareVersions(*version, current->GetVersion()) == 0) {
return true;
}
- } else if (!storedVersion && !currentVersion) {
+ } else if (!version && !current->HasVersion()) {
if (current->GetApplication() == tag) {
return true;
}
}
}
- if (useDefault) {
- if (current->HasVersion() && storedVersion) {
- auto currentVersion = current->GetVersion();
- if (!currentVersion.HasYear() || !storedVersion->HasYear()) {
- return true;
- }
- if (currentVersion.GetYear() != storedVersion->GetYear()) {
- errorReason = "Default rules used, stored's and current's Year differ, "
- + PrintStoredAndCurrent(stored, current);
- return false;
- }
- if (!currentVersion.HasMajor() || !storedVersion->HasMajor()) {
- return true;
- }
- if (std::abs((i32)currentVersion.GetMajor() - (i32)storedVersion->GetMajor()) <= 1) {
- return true;
- } else {
- errorReason = "Default rules used, stored's and current's Major difference is more than 1, "
- + PrintStoredAndCurrent(stored, current);
- return false;
- }
- } else if (!current->HasVersion() && !storedVersion) {
- if (*storedApplication == current->GetApplication()) {
- return true;
- } else {
- errorReason = "Default rules used, both versions are non-stable, stored's and current's Application differ, "
- + PrintStoredAndCurrent(stored, current);
- return false;
- }
+ // use common rule
+ if (current->HasVersion() && storedVersion) {
+ const auto& currentVersion = current->GetVersion();
+ if (!currentVersion.HasYear() || !storedVersion->HasYear()) {
+ return true;
+ }
+ if (currentVersion.GetYear() != storedVersion->GetYear()) {
+ errorReason = "Incompatible by common rule: peer's and current's Year differ, "
+ + PrintStoredAndCurrent(stored, current);
+ return false;
+ }
+ if (!currentVersion.HasMajor() || !storedVersion->HasMajor()) {
+ return true;
+ }
+ if (std::abs((i32)currentVersion.GetMajor() - (i32)storedVersion->GetMajor()) <= 1) {
+ return true;
} else {
- errorReason = "Default rules used, stable and non-stable versions are incompatible, "
+ errorReason = "Incompatible by common rule: peer's and current's Major differ by more than 1, "
+ PrintStoredAndCurrent(stored, current);
return false;
}
+ } else if (!current->HasVersion() && !storedVersion) {
+ if (*storedApplication == current->GetApplication()) {
+ return true;
+ } else {
+ errorReason = "Incompatible by common rule: both versions are non-stable, peer's and current's Build differ, "
+ + PrintStoredAndCurrent(stored, current);
+ return false;
+ }
+ } else {
+ errorReason = "Incompatible by common rule: one tag is stable and other is non-stable, "
+ + PrintStoredAndCurrent(stored, current);
+ return false;
}
- errorReason = "Version tag doesn't match any current compatibility rule, current version is not in accepted tags list, "
+ errorReason = "Peer version tag doesn't match any current compatibility rule, current version is not in accepted tags list, "
+ PrintStoredAndCurrent(stored, current);
return false;
}