From bd601c1ae519969c95f5f0540af8e5c53fcc6e84 Mon Sep 17 00:00:00 2001 From: Vasily Gerasimov Date: Fri, 5 Jan 2024 15:30:59 +0000 Subject: [PATCH] Get rid of lambdas in logging macros --- ydb/core/tx/columnshard/columnshard.cpp | 18 +++---- ydb/core/tx/ctor_logger.h | 53 +++++++++++++------ ydb/library/actors/core/log.h | 41 +++++++++----- .../dq/actors/compute/dq_compute_actor_impl.h | 18 +++---- 4 files changed, 77 insertions(+), 53 deletions(-) diff --git a/ydb/core/tx/columnshard/columnshard.cpp b/ydb/core/tx/columnshard/columnshard.cpp index 095db941991b..ba53d51fac2e 100644 --- a/ydb/core/tx/columnshard/columnshard.cpp +++ b/ydb/core/tx/columnshard/columnshard.cpp @@ -17,7 +17,7 @@ namespace NKikimr::NColumnShard { void TColumnShard::CleanupActors(const TActorContext& ctx) { ctx.Send(ResourceSubscribeActor, new TEvents::TEvPoisonPill); ctx.Send(BufferizationWriteActorId, new TEvents::TEvPoisonPill); - + StoragesManager->Stop(); if (Tiers) { Tiers->Stop(); @@ -307,10 +307,10 @@ void TColumnShard::ConfigureStats(const NOlap::TColumnEngineStats& indexStats, : tabletStats->SetRowCount(activeIndexStats.Rows); tabletStats->SetDataSize(activeIndexStats.Bytes + TabletCounters->Simple()[COUNTER_COMMITTED_BYTES].Get()); - + // TODO: we need row/dataSize counters for evicted data (managed by tablet but stored outside) //tabletStats->SetIndexSize(); // TODO: calc size of internal tables - + tabletStats->SetLastAccessTime(LastAccessTime.MilliSeconds()); tabletStats->SetLastUpdateTime(lastIndexUpdate.GetPlanStep()); } @@ -361,13 +361,13 @@ void TColumnShard::FillColumnTableStats(const TActorContext& ctx, std::unique_pt auto* periodicTableStats = ev->Record.AddTables(); periodicTableStats->SetDatashardId(TabletID()); periodicTableStats->SetTableLocalId(tableLocalID); - + periodicTableStats->SetShardState(2); // NKikimrTxDataShard.EDatashardState.Ready periodicTableStats->SetGeneration(Executor()->Generation()); periodicTableStats->SetRound(StatsReportRound++); periodicTableStats->SetNodeId(ctx.ExecutorThread.ActorSystem->NodeId); periodicTableStats->SetStartTime(StartTime().MilliSeconds()); - + if (auto* resourceMetrics = Executor()->GetResourceMetrics()) { resourceMetrics->Fill(*periodicTableStats->MutableTabletMetrics()); } @@ -376,11 +376,7 @@ void TColumnShard::FillColumnTableStats(const TActorContext& ctx, std::unique_pt FillTxTableStats(tableStats); ConfigureStats(*columnStats, tableStats); - { - // Workaround "reference to local binding declared in enclousing function" issue for clang14 - ui64 id = tableLocalID; - LOG_S_TRACE("Add stats for table, tableLocalID=" << id); - } + LOG_S_TRACE("Add stats for table, tableLocalID=" << tableLocalID); } } @@ -411,7 +407,7 @@ void TColumnShard::SendPeriodicStats() { FillOlapStats(ctx, ev); FillColumnTableStats(ctx, ev); - + NTabletPipe::SendData(ctx, StatsReportPipe, ev.release()); } diff --git a/ydb/core/tx/ctor_logger.h b/ydb/core/tx/ctor_logger.h index c09d2b051674..66437acd3a07 100644 --- a/ydb/core/tx/ctor_logger.h +++ b/ydb/core/tx/ctor_logger.h @@ -3,31 +3,52 @@ #include #include +#include + // You have to define TLogThis in local namespace to use these macros -#define LOG_S_EMERG(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_EMERG, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_ALERT(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_ALERT, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_CRIT(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_CRIT, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_ERROR(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_ERROR, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_WARN(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_WARN, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_NOTICE(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_NOTICE, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_INFO(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_INFO, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_DEBUG(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_DEBUG, [&](TStringBuilder& ss){ ss << stream; }); } -#define LOG_S_TRACE(stream) { TLogThis(*TlsActivationContext, NActors::NLog::PRI_TRACE, [&](TStringBuilder& ss){ ss << stream; }); } +#define __LOG_S_PRIORITY_IMPL__(stream, priority) \ + if (auto logThis = TLogThis(*TlsActivationContext, priority)) { \ + logThis.Str() << stream; \ + logThis.WriteLog(); \ + } + +#define LOG_S_EMERG(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_EMERG) +#define LOG_S_ALERT(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_ALERT) +#define LOG_S_CRIT(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_CRIT) +#define LOG_S_ERROR(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_ERROR) +#define LOG_S_WARN(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_WARN) +#define LOG_S_NOTICE(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_NOTICE) +#define LOG_S_INFO(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_INFO) +#define LOG_S_DEBUG(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_DEBUG) +#define LOG_S_TRACE(stream) __LOG_S_PRIORITY_IMPL__(stream, NActors::NLog::PRI_TRACE) namespace NKikimr { template class TCtorLogger { + const NActors::TActivationContext& Ctx; + NActors::NLog::EPriority Priority; + std::optional StrStream; + public: - template - TCtorLogger(const NActors::TActivationContext& ctx, NActors::NLog::EPriority priority, TFunc logFunc) + TCtorLogger(const NActors::TActivationContext& ctx, NActors::NLog::EPriority priority) + : Ctx(ctx) + , Priority(priority) { - if (IS_LOG_PRIORITY_ENABLED(priority, Component)) { - TStringBuilder strStream; - logFunc(strStream); - ::NActors::MemLogAdapter(ctx, priority, Component, "%s", strStream.data()); - } + } + + operator bool() const { + return IS_LOG_PRIORITY_ENABLED(Priority, Component); + } + + TStringBuilder& Str() { + StrStream.emplace(); + return *StrStream; + } + + void WriteLog() { + ::NActors::MemLogAdapter(Ctx, Priority, Component, std::move(*StrStream)); } }; diff --git a/ydb/library/actors/core/log.h b/ydb/library/actors/core/log.h index 18b21a031b09..6ce70d0b844d 100644 --- a/ydb/library/actors/core/log.h +++ b/ydb/library/actors/core/log.h @@ -25,11 +25,15 @@ // TODO: limit number of messages per second // TODO: make TLogComponentLevelRequest/Response network messages -#define IS_LOG_PRIORITY_ENABLED(priority, component) \ - [p = static_cast<::NActors::NLog::EPriority>(priority), c = static_cast<::NActors::NLog::EComponent>(component)]() -> bool { \ - ::NActors::TActivationContext *context = ::NActors::TlsActivationContext; \ - return !context || context->LoggerSettings()->Satisfies(p, c, 0ull); \ - }() +#define IS_LOG_PRIORITY_ENABLED(priority, component) \ + ::NActors::IsLogPriorityEnabled(static_cast<::NActors::NLog::EPriority>(priority), \ + static_cast<::NActors::NLog::EComponent>(component)) + +#define IS_CTX_LOG_PRIORITY_ENABLED(actorCtxOrSystem, priority, component, sampleBy) \ + ::NActors::IsLogPriorityEnabled(static_cast<::NActors::NLog::TSettings*>((actorCtxOrSystem).LoggerSettings()), \ + static_cast<::NActors::NLog::EPriority>(priority), \ + static_cast<::NActors::NLog::EComponent>(component), \ + sampleBy) #define IS_EMERG_LOG_ENABLED(component) IS_LOG_PRIORITY_ENABLED(NActors::NLog::PRI_EMERG, component) #define IS_ALERT_LOG_ENABLED(component) IS_LOG_PRIORITY_ENABLED(NActors::NLog::PRI_ALERT, component) @@ -43,21 +47,21 @@ #define LOG_LOG_SAMPLED_BY(actorCtxOrSystem, priority, component, sampleBy, ...) \ do { \ - ::NActors::NLog::TSettings* mSettings = static_cast<::NActors::NLog::TSettings*>((actorCtxOrSystem).LoggerSettings()); \ - ::NActors::NLog::EPriority mPriority = static_cast<::NActors::NLog::EPriority>(priority); \ - ::NActors::NLog::EComponent mComponent = static_cast<::NActors::NLog::EComponent>(component); \ - if (mSettings && mSettings->Satisfies(mPriority, mComponent, sampleBy)) { \ + if (IS_CTX_LOG_PRIORITY_ENABLED(actorCtxOrSystem, priority, component, sampleBy)) { \ ::NActors::MemLogAdapter( \ actorCtxOrSystem, priority, component, __VA_ARGS__); \ } \ } while (0) /**/ #define LOG_LOG_S_SAMPLED_BY(actorCtxOrSystem, priority, component, sampleBy, stream) \ - LOG_LOG_SAMPLED_BY(actorCtxOrSystem, priority, component, sampleBy, [&]() -> TString { \ - TStringBuilder logStringBuilder; \ - logStringBuilder << stream; \ - return std::move(logStringBuilder); \ - }()) + do { \ + if (IS_CTX_LOG_PRIORITY_ENABLED(actorCtxOrSystem, priority, component, sampleBy)) { \ + TStringBuilder logStringBuilder; \ + logStringBuilder << stream; \ + ::NActors::MemLogAdapter( \ + actorCtxOrSystem, priority, component, std::move(logStringBuilder)); \ + } \ + } while (0) /**/ #define LOG_LOG(actorCtxOrSystem, priority, component, ...) LOG_LOG_SAMPLED_BY(actorCtxOrSystem, priority, component, 0ull, __VA_ARGS__) #define LOG_LOG_S(actorCtxOrSystem, priority, component, stream) LOG_LOG_S_SAMPLED_BY(actorCtxOrSystem, priority, component, 0ull, stream) @@ -327,6 +331,15 @@ namespace NActors { } } // namespace NDetail + inline bool IsLogPriorityEnabled(NLog::TSettings* settings, NLog::EPriority proirity, NLog::EComponent component, ui64 sampleBy = 0) { + return settings && settings->Satisfies(proirity, component, sampleBy); + } + + inline bool IsLogPriorityEnabled(NLog::EPriority priority, NLog::EComponent component, ui64 sampleBy = 0) { + TActivationContext* context = TlsActivationContext; + return !context || IsLogPriorityEnabled(context->LoggerSettings(), priority, component, sampleBy); + } + template inline void DeliverLogMessage(TCtx& ctx, NLog::EPriority mPriority, NLog::EComponent mComponent, TString &&str) { diff --git a/ydb/library/yql/dq/actors/compute/dq_compute_actor_impl.h b/ydb/library/yql/dq/actors/compute/dq_compute_actor_impl.h index 9d477d407240..fd397b34951d 100644 --- a/ydb/library/yql/dq/actors/compute/dq_compute_actor_impl.h +++ b/ydb/library/yql/dq/actors/compute/dq_compute_actor_impl.h @@ -1572,10 +1572,9 @@ class TDqComputeActorBase : public NActors::TActorBootstrapped const auto& inputDesc = Task.GetInputs(inputIndex); Y_ABORT_UNLESS(inputDesc.HasSource()); source.Type = inputDesc.GetSource().GetType(); - const ui64 i = inputIndex; // Crutch for clang const auto& settings = Task.GetSourceSettings(); Y_ABORT_UNLESS(settings.empty() || inputIndex < settings.size()); - CA_LOG_D("Create source for input " << i << " " << inputDesc); + CA_LOG_D("Create source for input " << inputIndex << " " << inputDesc); try { std::tie(source.AsyncInput, source.Actor) = AsyncIoFactory->CreateDqSource( IDqAsyncIoFactory::TSourceArguments { @@ -1607,8 +1606,7 @@ class TDqComputeActorBase : public NActors::TActorBootstrapped std::tie(transform.InputBuffer, transform.Buffer) = TaskRunner->GetInputTransform(inputIndex); Y_ABORT_UNLESS(AsyncIoFactory); const auto& inputDesc = Task.GetInputs(inputIndex); - const ui64 i = inputIndex; // Crutch for clang - CA_LOG_D("Create transform for input " << i << " " << inputDesc.ShortDebugString()); + CA_LOG_D("Create transform for input " << inputIndex << " " << inputDesc.ShortDebugString()); try { std::tie(transform.AsyncInput, transform.Actor) = AsyncIoFactory->CreateDqInputTransform( IDqAsyncIoFactory::TInputTransformArguments { @@ -1644,8 +1642,7 @@ class TDqComputeActorBase : public NActors::TActorBootstrapped std::tie(transform.Buffer, transform.OutputBuffer) = TaskRunner->GetOutputTransform(outputIndex); Y_ABORT_UNLESS(AsyncIoFactory); const auto& outputDesc = Task.GetOutputs(outputIndex); - const ui64 i = outputIndex; // Crutch for clang - CA_LOG_D("Create transform for output " << i << " " << outputDesc.ShortDebugString()); + CA_LOG_D("Create transform for output " << outputIndex << " " << outputDesc.ShortDebugString()); try { std::tie(transform.AsyncOutput, transform.Actor) = AsyncIoFactory->CreateDqOutputTransform( IDqAsyncIoFactory::TOutputTransformArguments { @@ -1673,8 +1670,7 @@ class TDqComputeActorBase : public NActors::TActorBootstrapped const auto& outputDesc = Task.GetOutputs(outputIndex); Y_ABORT_UNLESS(outputDesc.HasSink()); sink.Type = outputDesc.GetSink().GetType(); - const ui64 i = outputIndex; // Crutch for clang - CA_LOG_D("Create sink for output " << i << " " << outputDesc); + CA_LOG_D("Create sink for output " << outputIndex << " " << outputDesc); try { std::tie(sink.AsyncOutput, sink.Actor) = AsyncIoFactory->CreateDqSink( IDqAsyncIoFactory::TSinkArguments { @@ -1829,15 +1825,13 @@ class TDqComputeActorBase : public NActors::TActorBootstrapped bool AllAsyncOutputsFinished() const { for (const auto& [outputIndex, sinkInfo] : SinksMap) { if (!sinkInfo.FinishIsAcknowledged) { - ui64 index = outputIndex; // Crutch for logging through lambda. - CA_LOG_D("Waiting finish of sink[" << index << "]"); + CA_LOG_D("Waiting finish of sink[" << outputIndex << "]"); return false; } } for (const auto& [outputIndex, transformInfo] : OutputTransformsMap) { if (!transformInfo.FinishIsAcknowledged) { - ui64 index = outputIndex; // Crutch for logging through lambda. - CA_LOG_D("Waiting finish of transform[" << index << "]"); + CA_LOG_D("Waiting finish of transform[" << outputIndex << "]"); return false; } }