Maintan schema versions reference count and remove unused from local database and memory #9134
Conversation
|
⚪ ⚪
🟢
*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 |
|
⚪
🟢
*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 |
aavdonkin
force-pushed
the
owner_in_txc
branch
from
207e53c
to
1c1c001
Compare
aavdonkin
force-pushed
the
owner_in_txc
branch
from
96b9609
to
3f56997
Compare
|
⚪ ⚪
🟢
*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 |
|
⚪
🟢
*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 |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
aavdonkin
force-pushed
the
owner_in_txc
branch
from
f3e0b8b
to
f2e6baf
Compare
|
⚪
🟢
*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 |
|
⚪
🟢
*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 |
|
⚪ ⚪
🟢
*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 |
aavdonkin
changed the title
|
⚪ ⚪
🟢
*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 |
| #pragma once | ||
|
|
||
| #if 0 | ||
| #define TEMPLOG(x) LOG_S_CRIT(x) |
There was a problem hiding this comment.
Для выборочной записи логов можно использовать AFL_DEBUG|TRACE|...
Как будто они решают ту же задачу -- зачем ещё один механизм?
| @@ -247,6 +251,7 @@ class TTransactionContext : public TTxMemoryProviderBase { | |||
| NTable::TDatabase &DB; | |||
| NWilson::TSpan &TransactionSpan; | |||
| NWilson::TSpan TransactionExecutionSpan; | |||
| NFlatExecutorSetup::ITablet* Owner = nullptr; | |||
There was a problem hiding this comment.
Зачем таблет в контексте транзакции?
Почему нельзя прокинуть CS, не нагружая им TTransactionContext? Но лучше даже не передавать CS, а передавать только счётчики туда, где они используются.
| @@ -620,6 +717,29 @@ class TColumnShard | |||
| } | |||
|
|
|||
| TColumnShard(TTabletStorageInfo* info, const TActorId& tablet); | |||
|
|
|||
| void VersionAddRef(ui64 portion, ui64 pathId, ui64 version) { | |||
There was a problem hiding this comment.
Счётчики стоит инкапсулировать в класс, чтобы не передавать CS везде, где нужен доступ к ним.
Как будто можно убрать эти методы и передавать VersionCounts везде.
| template<class Key, class Versions> | ||
| ui32 VersionRemoveRef(Versions& versions, const Key& portion, ui64 version) { | ||
| auto iter = versions.find(portion); | ||
| if (iter == versions.end()) { //Portion is already removed from local databae, no need to decrease ref count |
There was a problem hiding this comment.
А когда VersionRemoveRef может позваться дважды для одной порции? Почему возникает необходимость проверять, что порция не посчитана ранее?
То же самое для VersionAddRef.
| if (!PrimaryIndex) { | ||
| PrimaryIndex = std::make_unique<NOlap::TColumnEngineForLogs>(TabletId, StoragesManager, version, schema); | ||
| PrimaryIndex = std::make_unique<NOlap::TColumnEngineForLogs>(TabletId, StoragesManager, version, schema, CS, &db); |
There was a problem hiding this comment.
У db время жизни ограничено фазой Execute транзакции, а ссылка на неё сохраняется в TColumnEngineForLogs, который переживёт транзакцию. TColumnEngineForLogs не должен взаимодействовать с локальной базой вне транзакции, а если он так не делает, то можно не хранить в нём ссылку.
| @@ -95,6 +120,9 @@ THashSet<TInsertWriteId> TInsertTable::OldWritesToAbort(const TInstant& now) con | |||
| void TInsertTable::EraseCommittedOnExecute( | |||
| IDbWrapper& dbTable, const TCommittedData& data, const std::shared_ptr<IBlobsDeclareRemovingAction>& blobsAction) { | |||
| if (Summary.HasCommitted(data)) { | |||
| dbTable.OnCommit([cs = CS, planStep = data.GetSnapshot().GetPlanStep(), txId = data.GetSnapshot().GetTxId(), pathId = data.GetPathId(), dedupId = data.GetDedupId(), schema = data.GetSchemaVersion()]() { | |||
There was a problem hiding this comment.
А можно обновить счётчики в таком месте, чтобы было очевидно, что удаление порции из памяти ровно один раз декрементирует счётчик и аналогично для добавления порции? Я не вчитывался, но как будто это можно сделать в TGranuleMeta или рядом с ней.
Сейчас, мне кажется, есть две проблемы:
- Чтобы не обновить счётчик дважды для одной порции, приходится дублировать список существующих порций в VersionCounts. Непонятно, почему нельзя избежать этих проверок.
- Не очевидно, что удаление порции всегда приводит к декременту счётчика и добавление -- к инкременрту. Как будто эту гарантию легко случайно нарушить, меняя код.
Changelog entry
...
Changelog category
Additional information
...