Workflow [PR], commit [97e95fa]

Summary: ❌

Thanks @Algunenano, decent comments!

Could you please leave a comment explaining why it's done. It's obvious to me now, but probably not to everyone and not me in a few months.

Sure

I think we can remove MemoryTrackerDebugBlockerInThread blocker;
since a) it's already covered here and b) it's also covered by SystemLogQueue::push.

The reason for it is very different, it is to avoid polluting trace_log with MemoryAllocatedWithoutCheck (see 4117df8 and this is the case of resize only actually, not the log element itself, which allocations should not be accounted in query context as well). Let me put a comment as well.

Could this have been an issue inside sync logging when calling pushExtendedMessageToInternalTCPTextLogQueue? It seems it might grow the queue, allocating extra memory that's not freed but it's tied to CurrentThread so probably accounting was kept correctly.

Exactly.

This makes me wonder what happens in logging threads if an exception is thrown when logging, inside while (is_open). It seems the thread might stop and never recover. I'll investigate further

Likely, but the memory exceptions are blocked in that function.

Could this have been an issue inside sync logging when calling pushExtendedMessageToInternalTCPTextLogQueue? It seems it might grow the queue, allocating extra memory that's not freed but it's tied to CurrentThread so probably accounting was kept correctly.

Actually I think for this case it should be accounted properly, since TCPHandler (and HTTPHandler) has CurrentThread::QueryScope which creates initial thread group for a query

Likely, but the memory exceptions are blocked in that function.

Yes, but I'm guessing there might be other functions, like a temporal disk failure or something similar (thinking about a remote syslog for example). I'll do some more digging and probably cover it with a try...catch(...) just in case

Integration tests (amd_tsan, 2/6)

• test_storage_kafka - data-race in librdkafka (rd_kafka_toppar_leader_update vs rd_kafka_broker_fetch_toppars) #87654

Stateless tests (amd_binary, ParallelReplicas, s3 storage, parallel)

• Fuzzing 02950_part_offset_as_primary_key.sql with parallel replicas #87653

Integration tests (arm_binary, distributed plan, 4/4)

• test_keeper_memory_soft_limit/test.py::test_soft_limit_create - will take a look - fixed in 9e9c849

Integration tests (amd_binary, 4/5)

• tests: fix flakiness of integration tests due to too poor logs checks #87719

Integration tests (arm_binary, distributed plan, 1/4)

• Flaky integration test test_storage_rabbitmq #71049

There can be some other issues (since we seen that memory usage was very low, so likely we have some places where we allocate with blocked memory tracker, or from another context, but free w/o blocking tracker), but, those will be done separately, this was the major one, others should be less significant (if any). Merging.

Likely, but the memory exceptions are blocked in that function.
Yes, but I'm guessing there might be other functions, like a temporal disk failure or something similar (thinking about a remote syslog for example). I'll do some more digging and probably cover it with a try...catch(...) just in case

I should have done this before: #88814