-
Notifications
You must be signed in to change notification settings - Fork 727
Move MaxTxInFly to ICB #27635
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
Move MaxTxInFly to ICB #27635
Conversation
|
🟢 |
73e0fe8 to
1ce5967
Compare
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/core/base/appdata.cpp
Outdated
| NKikimrConfig::TStatisticsConfig StatisticsConfig; | ||
| TMetricsConfig MetricsConfig; | ||
| NKikimrConfig::TSystemTabletBackupConfig SystemTabletBackupConfig; | ||
| NKikimrConfig::TSystemTabletConfig SystemTabletConfig; |
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.
я бы перенёс SystemTabletBackupConfig в SystemTabletConfig, например как поле BackupConfig
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.
От этого же поедут существующие конфиги. По хорошему, переносы делаются так:
- Заводим новые поля и связываем их со старыми
- Старые объявляем deprecated
- Ждем слудующей версии
- Выдаем ошибку при использовании старых
- Ждем следующей версии
- Удаляем старые
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.
В общем, ИМХО оно того не стоит в данном случае.
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.
ладно, перенесём потом если поймем что можно
ydb/core/protos/config.proto
Outdated
| optional TStatisticsConfig StatisticsConfig = 113; | ||
| optional TMetricsConfig MetricsConfig = 114; | ||
| optional TSystemTabletBackupConfig SystemTabletBackupConfig = 115; | ||
| optional TSystemTabletConfig SystemTabletConfig = 116; |
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.
Обсудили в личке, SystemTablet намекает, что это про "системные таблетки", которыми у нас принято называть кластерные таблетки типа bscontroller'а, schemeshard'а и т.д. Предложил перенести настройку в icb (рядом с MaxCommitRedoMB), чтобы её можно было в том числе в рантайме крутить.
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
Move MaxTxInFly to separate config TSystemTabletConfig
Changelog category
Description for reviewers
(refactoring after #26783)