-
Notifications
You must be signed in to change notification settings - Fork 37
Add api and functionality to compare disk registry state with db record #4842
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi! Thank you for contributing! |
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f974118.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f974118.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f974118.
|
cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_compare_state.cpp
Outdated
Show resolved
Hide resolved
| TTxDiskRegistry::TCompareDiskRegistryState& args) | ||
| { | ||
| Y_UNUSED(ctx); | ||
| Y_UNUSED(args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишнее
cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_compare_state.cpp
Outdated
Show resolved
Hide resolved
cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_compare_state.cpp
Outdated
Show resolved
Hide resolved
cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_compare_state.cpp
Outdated
Show resolved
Hide resolved
| static_assert(sizeof(*this) == 2144); | ||
|
|
||
| google::protobuf::util::MessageDifferencer diff; | ||
| NProtoBuf::string report; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::string?
| TVector<TString> result; | ||
|
|
||
| if(!diff.Compare(StorageConfig->GetStorageConfigProto(), rhs.StorageConfig->GetStorageConfigProto())) { | ||
| std::stringstream ss; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TStringBuilder
| } | ||
|
|
||
| if(!diff.Compare(CurrentConfig, rhs.CurrentConfig)) { | ||
| std::stringstream ss; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TStringBuilder
| } | ||
|
|
||
| const auto& vPlacementGroups = rhs.PlacementGroups; | ||
| std::stringstream ss; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TStringBuilder
|
|
||
| TVector<TLaggingDevice> OutdatedLaggingDevices; | ||
|
|
||
| TString getDiff(const TDiskState& rhs) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
С большой буквы названия
| xxx(GetAgentNodeId, __VA_ARGS__) \ | ||
| xxx(AddOutdatedLaggingDevices, __VA_ARGS__) \ | ||
| xxx(GetClusterCapacity, __VA_ARGS__) \ | ||
| xxx(CompareDiskRegistryStateWithLocalDb, __VA_ARGS__) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поздно заметил, со следующими правками исправлю
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit d172d31.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit d172d31.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit d172d31.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 6985ce8.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 6985ce8.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 6985ce8.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 27b77de.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 27b77de.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 27b77de.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f898a19.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f898a19.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f898a19.
|
950ceb8 to
54885f3
Compare
| using namespace NKikimr::NTabletFlatExecutor; | ||
|
|
||
| namespace { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//////
| STFUNC(TCompareActor::StateCompare) | ||
| { | ||
| switch (ev->GetTypeRewrite()) { | ||
| HFunc( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
надо бы обрабатывать PoisonPill, чтобы если DR умрет, то и этот актор умер
| TransactionTimeTracker.GetInflightInfo(GetCycleCount()).c_str()); | ||
|
|
||
| NProto::TBackupDiskRegistryStateResponse record; | ||
| if (msg->Record.GetSource() != NProto::EBackupDiskRegistryStateSource::LOCAL_DB) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (msg->Record.GetSource() != NProto::EBackupDiskRegistryStateSource::LOCAL_DB) { | |
| if (msg->Record.GetSource() == NProto::EBackupDiskRegistryStateSource::RAM) { |
| std::move(requestInfo), | ||
| msg->Record.GetBackupFilePath()); | ||
| msg->Record.GetBackupFilePath(), | ||
| record); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зочем?
| Y_UNUSED(ctx); | ||
|
|
||
| TDiskRegistryDatabase db(tx.DB); | ||
| return LoadState(db, args.Snapshot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лучше сюда добавить
if (args.source == NProto::EBackupDiskRegistryStateSource::BOTH) {
args.RamSnapshot = State->BackupState();
}
| ui64 diskStateSeqNo, | ||
| TVector<TDiskId> outdatedVolumes); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
убери, и пройдись по README и включи в вскоде все настройки вскода нужные чтобы таких space ов не было на пустых строках
| UNIT_ASSERT(!diskRegistry.Exists("nonrepl-garbage")); | ||
| } | ||
|
|
||
| Y_UNIT_TEST(ShouldKeepDiskRegistryStateEqualWithLocalDb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
стоит добавить тест что отлавливается разъезд локальной базы и стейта
| message TBackupDiskRegistryStateRequest | ||
| enum EBackupDiskRegistryStateSource | ||
| { | ||
| RAM = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
скорее что-то тип
BACKUP_DISK_REGISTRY_STATE_SOURCE_RAM
и остальные значения енума анологично
| xxx(PurgeHostCms, __VA_ARGS__) \ | ||
| xxx(RemoveOrphanDevices, __VA_ARGS__) \ | ||
| xxx(AddOutdatedLaggingDevices, __VA_ARGS__) \ | ||
| xxx(CompareDiskRegistryStateWithLocalDb, __VA_ARGS__) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
можно убрать эту транзакцию теперь
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 2f845a0.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 2f845a0.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 2f845a0.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e6d8420.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e6d8420.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e6d8420.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 97c4e58.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 97c4e58.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 97c4e58.
|
No description provided.