Why I'm doing:
Now we initialize the column evaluator in the partition writer, which cause the same evaluator instance may be modified by different threads.
What I'm doing:
Remove the unnecessary evaluator initialization in the iceberg partition writer because it has been initialized in partition chunk writer factory.
Also, we check the file writer and create it if needed before writing file data, because it may has been reset by the previous commit operation.
Fixes #issue
Signed-off-by: GavinMar <yangguansuo@starrocks.com>
## Why I'm doing
- ASAN reported a memory leak in BE unit test `LocalTabletsChannelTest::test_add_chunk_not_exist_tablet`
- Direct leak from `SecondaryReplicasWaiter::_send_replica_status_request()` allocating `ReusableClosure<PLoadReplicaStatusResult>`
```
=================================================================
==752488==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 792 byte(s) in 1 object(s) allocated from:
#0 0x1a830f48 in operator new(unsigned long) (/root/celerdata/be/ut_build_ASAN/test/starrocks_test+0x1a830f48)
#1 0x30153219 in starrocks::SecondaryReplicasWaiter::_send_replica_status_request(int) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1342
#2 0x30152d52 in starrocks::SecondaryReplicasWaiter::_try_check_replica_status_on_primary(int) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1327
#3 0x301520e3 in starrocks::SecondaryReplicasWaiter::wait() /root/celerdata/be/src/runtime/local_tablets_channel.cpp:1285
#4 0x3013603a in starrocks::LocalTabletsChannel::add_chunk(starrocks::Chunk*, starrocks::PTabletWriterAddChunkRequest const&, starrocks::PTabletWriterAddBatchResult*, bool*) /root/celerdata/be/src/runtime/local_tablets_channel.cpp:380
#5 0x1eefff41 in starrocks::LocalTabletsChannelTest_test_add_chunk_not_exist_tablet_Test::TestBody() /root/celerdata/be/test/runtime/local_tablets_channel_test.cpp:255
#6 0x37d95703 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2433
#7 0x37d95703 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2469
#8 0x37d8856d in testing::Test::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2508
#9 0x37d8856d in testing::Test::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2498
#10 0x37d886cd in testing::TestInfo::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2684
#11 0x37d886cd in testing::TestInfo::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2657
#12 0x37d887b6 in testing::TestSuite::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2816
#13 0x37d887b6 in testing::TestSuite::Run() googletest-release-1.10.0/googletest/src/gtest.cc:2795
#14 0x37d88d13 in testing::internal::UnitTestImpl::RunAllTests() googletest-release-1.10.0/googletest/src/gtest.cc:5338
#15 0x37d88f23 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2433
#16 0x37d88f23 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) googletest-release-1.10.0/googletest/src/gtest.cc:2469
#17 0x37d88f23 in testing::UnitTest::Run() googletest-release-1.10.0/googletest/src/gtest.cc:4925
#18 0x1a893ea2 in RUN_ALL_TESTS() /var/local/thirdparty/installed/include/gtest/gtest.h:2473
#19 0x1a87d8a4 in starrocks::init_test_env(int, char**) /root/celerdata/be/src/testutil/init_test_env.h:115
#20 0x1a87e332 in main /root/celerdata/be/test/test_main.cpp:18
#21 0x7f5813e29d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
```
- Root cause:
- The send path does `closure->ref()` twice (one for “self-hold”, one for “RPC-hold”).
- The test does not register a sync-point callback for `LocalTabletsChannel::rpc::get_load_replica_status` to simulate the RPC, so no one calls `closure->Run()`, and the “RPC-hold” ref is never released.
- The waiter’s release path only dropped one ref, leaving one outstanding reference and causing the leak.
## What I'm doing
- Introduce and use a sync-point primitive `TEST_SYNC_POINT_CALLBACK_OR_DEFAULT` with a default action, so tests without a registered callback still release the “RPC-hold” reference.
- At `LocalTabletsChannel::rpc::get_load_replica_status`, switch to a callback-with-default that calls `closure->Run()` when no test callback is installed. This balances the double `ref()` even in no-callback tests, while keeping production behavior unchanged.
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: Kevin Cai <kevin.cai@celerdata.com>
lyw
Murphy
eyes_on_me
Drake Wang
satanson
Evgeniy Shishkin
meegoo
kangkaisen
Yixin Luo
Gavin
yan zhang
trueeyu
stdpain
IHEII
zihe.liu
zhanghe
zhangqiang
PengFei Li
stephen
predator4ann
srlch
nillin