-
Notifications
You must be signed in to change notification settings - Fork 610
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
add missing table stats for column shards #7140
Conversation
ed8764e
to
2e29022
Compare
31faa1f
to
893089b
Compare
namespace NKikimr::NColumnShard { | ||
|
||
void TTableStatsBuilder::FillColumnTableStats(const TSingleColumnTableCounters& stats) { | ||
stats.FillStats(TableStats); |
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.
не знаю, насколько хорошо уносить заполнение частей протобуфа в каунтеры
@@ -1,6 +1,8 @@ | |||
#pragma once | |||
#include "engines/changes/abstract/compaction_info.h" | |||
#include "engines/portions/meta.h" | |||
#include <ydb/core/tx/columnshard/counters/statistics_store.h> | |||
#include <ydb/core/base/appdata.h> |
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.
эту зависимость, лучше, в h-файлы не тянуть
bool ActiveCleanupPortions = false; | ||
bool ActiveCleanupTables = false; | ||
bool ActiveCleanupInsertTable = false; | ||
YDB_READONLY(TMonotonic, LastIndexationInstant, TMonotonic::Zero()); | ||
public: | ||
TBackgroundController(TBackgroundControllerCounters& stats) : Stats(stats) { |
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.
на разных строчках по style guide
@@ -15,11 +17,15 @@ class TBackgroundController { | |||
using TCurrentCompaction = THashMap<ui64, NOlap::TPlanCompactionInfo>; | |||
TCurrentCompaction ActiveCompactionInfo; | |||
|
|||
TBackgroundControllerCounters& Stats; |
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::shared_ptr<>
} | ||
} | ||
|
||
TInstant GetLastCompactionFinishInstant(ui64 pathId) 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.
const ui64 pathId рекомендую
и в private, поскольку не используется снаружи
} | ||
} | ||
|
||
TInstant GetLastCompactionFinishInstant(ui64 pathId) 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.
лучше отдавать std::optional
|
||
namespace NKikimr::NColumnShard { | ||
|
||
class TStatisticsStore { |
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.
TCountersManager
@@ -193,17 +193,17 @@ TConclusionStatus TGeneralCompactColumnEngineChanges::DoConstructBlobs(TConstruc | |||
void TGeneralCompactColumnEngineChanges::DoWriteIndexOnComplete(NColumnShard::TColumnShard* self, TWriteIndexCompleteContext& context) { | |||
TBase::DoWriteIndexOnComplete(self, context); | |||
if (self) { | |||
self->IncCounter( | |||
self->Stats.GetTabletCounters().IncCounter( |
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.
лучше сагрегировать в 1 метод
self->IncCounter(NColumnShard::COUNTER_INDEXING_BLOBS_WRITTEN, context.BlobsWritten); | ||
self->IncCounter(NColumnShard::COUNTER_INDEXING_BYTES_WRITTEN, context.BytesWritten); | ||
self->IncCounter(NColumnShard::COUNTER_INDEXING_TIME, context.Duration.MilliSeconds()); | ||
self->Stats.GetTabletCounters().IncCounter(NColumnShard::COUNTER_INDEXING_BLOBS_WRITTEN, context.BlobsWritten); |
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.
в 1 метод
Self->IncCounter(NColumnShard::COUNTER_READ_INDEX_BLOBS, statsDelta.Blobs); | ||
Self->IncCounter(NColumnShard::COUNTER_READ_INDEX_ROWS, statsDelta.Rows); | ||
Self->IncCounter(NColumnShard::COUNTER_READ_INDEX_BYTES, statsDelta.Bytes); | ||
Self->Stats.GetTabletCounters().IncCounter(NColumnShard::COUNTER_READ_INDEX_PORTIONS, statsDelta.Portions); |
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.
1 метод
owner.IncCounter(COUNTER_BLOBS_COMMITTED, counters.Rows); | ||
owner.IncCounter(COUNTER_BYTES_COMMITTED, counters.Bytes); | ||
owner.IncCounter(COUNTER_RAW_BYTES_COMMITTED, counters.RawBytes); | ||
owner.Stats.GetTabletCounters().IncCounter(COUNTER_BLOBS_COMMITTED, counters.Rows); |
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.
1 метод
893089b
to
7a579d0
Compare
f069a43
to
0bdba01
Compare
e617484
to
5b8f168
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 |
wBuffer.RemoveData(aggr, StoragesManager->GetInsertOperator()); | ||
continue; | ||
} | ||
|
||
if (putResult.GetPutStatus() != NKikimrProto::OK) { | ||
CSCounters.OnWritePutBlobsFail(TMonotonic::Now() - writeMeta.GetWriteStartInstant()); | ||
IncCounter(COUNTER_WRITE_FAIL); | ||
Counters.GetCSCounters().OnWritePutBlobsFail(TMonotonic::Now() - writeMeta.GetWriteStartInstant()); |
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.
объединить
@@ -162,6 +156,8 @@ void TColumnShard::Handle(TEvColumnShard::TEvWrite::TPtr& ev, const TActorContex | |||
const TString dedupId = record.GetDedupId(); | |||
const auto source = ev->Sender; | |||
|
|||
Counters.GetColumnTablesCounters()->GetPathIdCounter(tableId)->OnUpdate(); |
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.
либо OnWriteEvent либо UpdateLastEventTime
@@ -379,7 +375,7 @@ void TColumnShard::Handle(NEvents::TDataEvents::TEvWrite::TPtr& ev, const TActor | |||
return; | |||
} | |||
|
|||
auto wg = WritesMonitor.RegisterWrite(arrowData->GetSize()); | |||
auto wg = Counters.GetWritesMonitor()->OnStartWrite(arrowData->GetSize()); |
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.
On*** это методы-сообщения, возврат значений из которых не предполагается
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.
попробовать удалить Guard. сделать UpdateCounters сразу
auto* tabletStats = ev->Record.MutableTableStats(); | ||
FillTxTableStats(tabletStats); | ||
|
||
TTableStatsBuilder statsBuilder(*ev->Record.MutableTableStats()); |
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.
давай сделаем тогда так
TTableStatsBuilder statsFiller(counters, executor, columnsEngine);
statsFiller.SerializeToProto(*ev->Record.MutableTableStats());
тогда при добавлении объектов все будет происходить локально внутри TTableStatsBuilder
- Counters заполняют свои поля самостоятельно ОДНИМ методом из Counters
auto* tableStats = periodicTableStats->MutableTableStats(); | ||
FillTxTableStats(tableStats); | ||
ConfigureStats(*columnStats, tableStats); | ||
TTableStatsBuilder statsBuilder(*periodicTableStats->MutableTableStats()); |
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.
Аналогично пред. комментарию.
ConfigureStats(*columnStats, tableStats); | ||
TTableStatsBuilder statsBuilder(*periodicTableStats->MutableTableStats()); | ||
statsBuilder.FillColumnTableStats(*Counters.GetColumnTablesCounters()->GetPathIdCounter(pathId)); | ||
statsBuilder.FillTabletStats(*Counters.GetTabletCounters()); |
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.
пока-что, лучше, данные по таблице, не относящиеся к таблице убрать.
Track missing table stats in column shard similarly to data shard. New stats are available in DescribeTable.