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:
For **COM_STMT_EXECUTE**, `ConnectProcessor.handleExecute` sets
`ctx.getState().setIsQuery(ctx.isQueryStmt(executeStmt))`,
but this is later overwritten by
`context.getState().setIsQuery(parsedStmt instanceof QueryStatement)` inside `executor.execute()`.
Signed-off-by: zihe.liu <ziheliu1024@gmail.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>
Cursor Agent
eyes_on_me
Yixin Luo
Murphy
絵空事スピリット
zhanghe
Harbor Liu
Youngwb
Qian Ye
Gavin
shuming.li
zhangqiang
satanson
yan zhang
before-Sunrise
kangkaisen
stephen
predator4ann
SevenJ
zihe.liu
PengFei Li
yandongxiao
Seaven