Ydb topics kafka proxy add metadata commit fetch actors #17932
Conversation
rename TConsumerOffset struct add PartitionIndex field to TConsumerOffset struct to substitude std::pair in OffsetCommit function
nerhneiro
requested review from
a-serebryanskiy
and removed request for
alexnick88
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
| std::unordered_map<ui32, ui32> PartitionIdToOffset {}; | ||
| std::shared_ptr<std::unordered_map<ui32, std::unordered_map<TString, ui32>>> PartitionIdToOffsets = std::make_shared<std::unordered_map<ui32, std::unordered_map<TString, ui32>>>(); | ||
| std::unordered_map<ui32, TEvKafka::PartitionGroupOffset> PartitionIdToOffset {}; | ||
| std::shared_ptr<std::unordered_map<ui32, std::unordered_map<TString, TEvKafka::PartitionGroupOffset>>> PartitionIdToOffsets = std::make_shared<std::unordered_map<ui32, std::unordered_map<TString, TEvKafka::PartitionGroupOffset>>>(); |
There was a problem hiding this comment.
А вот тут обрати внимание: если я правильно понял, то std::unordered_map<TString, ui32> - это мепа имя консьмер группы к оффсету. Но ведь имя консьюмер группы у нас уже есть в новом объекте TConsumerOffset, верно? Тогда зачем нам его дублировать.
P.S. замечаю, что не первый раз даю комментарий по рефакторингу существующего кода. Не стесняйся плз рефакторить существующий код в будущем, если после твоих изменений он становится не актуальным.
ydb/core/kafka_proxy/kafka_events.h
Outdated
| std::shared_ptr<std::unordered_map<ui32, std::unordered_map<TString, ui32>>> PartitionIdToOffsets; | ||
|
|
||
| // std::shared_ptr<std::unordered_map<ui32, std::unordered_map<TString, ui32>>> PartitionIdToOffsets; | ||
| std::shared_ptr<std::unordered_map<ui32, std::unordered_map<TString, PartitionGroupOffset>>> PartitionIdToOffsets; |
There was a problem hiding this comment.
Здесь тоже давай подумаем, нужна ли нам до сих пор мэпа, сходу не уверен, посмотри плз и отпишись комментом, что и почему решила: оставояем мепу или переделываем на вектор с объектами.
There was a problem hiding this comment.
В методе void Reply(const TActorContext& ctx) override
в поле PartitionIdToOffsets объекта response передается PartitionIdToOffsets из класса TTopicOffsetActor. Чтобы не тратить время на изменение типа данных, удобнее хранить PartitionIdToOffsets в TEvCommitedOffsetsResponse такого же типа, как в TTopicOffsetActor.
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
Changelog entry
Add CommitedMetadata to OffsetFetchActor and OffsetCommitActor. Add metadata field to PQ-tablet event.
Changelog category
Description for reviewers
...