Skip to content

Range size tracking #420

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.

Sign up for GitHub

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

Closed
yi-xmu wants to merge 15 commits into from
Closed

Range size tracking #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8bf4940
range sizes definition
yi-xmu Feb 25, 2026
ff06fc1
update range size on postwritecc
yi-xmu Feb 25, 2026
15cb6ad
update store range size after datasync
yi-xmu Feb 26, 2026
575f24d
clang format
yi-xmu Feb 26, 2026
4a6a18e
Init range sizes for ccmap
yi-xmu Feb 26, 2026
f072cc8
update
yi-xmu Feb 27, 2026
24d92a9
update the definition of range sizes
yi-xmu Feb 28, 2026
772a6b1
modify the interface of update range size
yi-xmu Feb 28, 2026
4360e6e
modify logic for load range size
yi-xmu Feb 28, 2026
83e6a2e
update
yi-xmu Feb 28, 2026
05a1c3a
reset range size after range split
yi-xmu Mar 1, 2026
76fa3d1
set range size flags for postwrite request
yi-xmu Mar 1, 2026
4694818
update
yi-xmu Mar 2, 2026
2330266
update range size during create secondary index
yi-xmu Mar 4, 2026
6a2d770
update delta size of ccentry
yi-xmu Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions store_handler/bigtable_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,13 @@ void EloqDS::BigTableHandler::FetchRangeSlices(
fetch_cc));
}

void EloqDS::BigTableHandler::FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "BigTableHandler::FetchTableRangeSize not implemented";
assert(false);
}
Comment on lines +713 to +718
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Complete the CC request instead of aborting the process.

Line 717 uses assert(false). That crashes debug builds, and in release builds the assert is removed so fetch_cc is never finished, which can leave the caller hanging indefinitely.

Proposed fix 
 void EloqDS::BigTableHandler::FetchTableRangeSize(
     txservice::FetchTableRangeSizeCc *fetch_cc)
 {
     LOG(ERROR) << "BigTableHandler::FetchTableRangeSize not implemented";
-    assert(false);
+    fetch_cc->SetFinish(static_cast<int>(txservice::CcErrorCode::DATA_STORE_ERR));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void EloqDS::BigTableHandler::FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "BigTableHandler::FetchTableRangeSize not implemented";
assert(false);
}
void EloqDS::BigTableHandler::FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "BigTableHandler::FetchTableRangeSize not implemented";
fetch_cc->SetFinish(static_cast<int>(txservice::CcErrorCode::DATA_STORE_ERR));
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/bigtable_handler.cpp` around lines 713 - 718, The
FetchTableRangeSize method currently aborts via assert(false) and never
completes the txservice::FetchTableRangeSizeCc request; instead, remove the
assert and implement a proper error completion path: log the unimplemented path,
set an appropriate error status on fetch_cc (e.g., an internal/NOT_IMPLEMENTED
error or populate any available response/error fields on
txservice::FetchTableRangeSizeCc), then call fetch_cc->Complete() (or the
correct completion method) so the caller is always notified; reference the
EloqDS::BigTableHandler::FetchTableRangeSize function and the fetch_cc parameter
when making these changes.


void EloqDS::BigTableHandler::OnFetchRangeSlices(
google::cloud::future<google::cloud::StatusOr<
std::pair<bool, google::cloud::bigtable::Row>>> f,
Expand Down
3 changes: 3 additions & 0 deletions store_handler/bigtable_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class BigTableHandler : public txservice::store::DataStoreHandler

void FetchRangeSlices(txservice::FetchRangeSlicesReq *fetch_cc) override;

void FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc) override;

Comment on lines +85 to +87
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n -A 15 'BigTableHandler::FetchTableRangeSize' --type=cpp

Repository: eloqdata/tx_service

Length of output: 1492

Fix stub implementation to properly signal unsupported operation.

The stub implementation at lines 713-718 in bigtable_handler.cpp currently uses assert(false) and does not interact with the fetch_cc parameter. This approach has two problems:

  1. Silent failure in release builds: With assertions disabled, the function returns without setting any error status on fetch_cc, leaving the caller with an uninitialized/unmodified object.
  2. No proper error signaling: Unlike proper error handling patterns in the codebase, this stub doesn't set an appropriate error code or failure status on the completion callback.

Set an appropriate error status on fetch_cc (e.g., fetch_cc->set_error(...)) before returning, or use a structured failure response, to ensure callers are properly notified that this operation is not yet supported.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/bigtable_handler.h` around lines 85 - 87, The stub
FetchTableRangeSize implementation must stop using assert(false) and instead set
an explicit error on the completion callback parameter
txservice::FetchTableRangeSizeCc *fetch_cc to signal an unsupported operation;
locate the FetchTableRangeSize definition in bigtable_handler.cpp and replace
the assert with a call that sets an appropriate error/status on fetch_cc (for
example using fetch_cc->set_error(...) or the project's equivalent
set_status/failure method with a clear "not implemented/unsupported" message)
before returning so callers always receive a structured failure response.

/**
* @brief Read a row from base table or skindex table in datastore with
* specified key. Caller should pass in complete primary key or skindex key.
Expand Down
48 changes: 43 additions & 5 deletions store_handler/data_store_service_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,30 @@ void DataStoreServiceClient::FetchRangeSlices(
&FetchRangeSlicesCallback);
}

void DataStoreServiceClient::FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc)
{
txservice::TableName range_table_name(fetch_cc->table_name_->StringView(),
txservice::TableType::RangePartition,
fetch_cc->table_name_->Engine());

int32_t kv_partition_id =
KvPartitionIdOfRangeSlices(range_table_name, fetch_cc->partition_id_);
uint32_t shard_id = GetShardIdByPartitionId(kv_partition_id, false);

auto catalog_factory = GetCatalogFactory(range_table_name.Engine());
assert(catalog_factory != nullptr);
fetch_cc->kv_start_key_ =
EncodeRangeKey(catalog_factory, range_table_name, fetch_cc->start_key_);

Read(kv_range_table_name,
kv_partition_id,
shard_id,
fetch_cc->kv_start_key_,
fetch_cc,
&FetchRangeSizeCallback);
}

/**
* @brief Deletes data that is out of the specified range.
*
Expand Down Expand Up @@ -1254,16 +1278,19 @@ std::string DataStoreServiceClient::EncodeRangeKey(
* @param range_version The version of the range.
* @param version The general version number.
* @param segment_cnt The number of segments in the range.
* @param range_size The size of the range.
* @return Binary string containing the encoded range value.
*/
std::string DataStoreServiceClient::EncodeRangeValue(int32_t range_id,
uint64_t range_version,
uint64_t version,
uint32_t segment_cnt)
uint32_t segment_cnt,
int32_t range_size)
{
std::string kv_range_record;
kv_range_record.reserve(sizeof(int32_t) + sizeof(uint64_t) +
sizeof(uint64_t) + sizeof(uint32_t));
sizeof(uint64_t) + sizeof(uint32_t) +
sizeof(int32_t));
kv_range_record.append(reinterpret_cast<const char *>(&range_id),
sizeof(int32_t));
kv_range_record.append(reinterpret_cast<const char *>(&range_version),
Expand All @@ -1273,6 +1300,8 @@ std::string DataStoreServiceClient::EncodeRangeValue(int32_t range_id,
// segment_cnt of slices
kv_range_record.append(reinterpret_cast<const char *>(&segment_cnt),
sizeof(uint32_t));
kv_range_record.append(reinterpret_cast<const char *>(&range_size),
sizeof(int32_t));
return kv_range_record;
}

Expand Down Expand Up @@ -1340,6 +1369,7 @@ RangeSliceBatchPlan DataStoreServiceClient::PrepareRangeSliceBatches(
RangeSliceBatchPlan plan;
plan.segment_cnt = 0;
plan.version = version;
plan.range_size = 0;

// Estimate capacity based on slices size
plan.segment_keys.reserve(slices.size() / 10 + 1); // Rough estimate
Expand Down Expand Up @@ -1388,6 +1418,7 @@ RangeSliceBatchPlan DataStoreServiceClient::PrepareRangeSliceBatches(
sizeof(uint32_t));
segment_record.append(slice_start_key.Data(), key_size);
uint32_t slice_size = static_cast<uint32_t>(slices[i]->Size());
plan.range_size += static_cast<int32_t>(slice_size);
segment_record.append(reinterpret_cast<const char *>(&slice_size),
sizeof(uint32_t));
}
Expand Down Expand Up @@ -1553,6 +1584,7 @@ void DataStoreServiceClient::EnqueueRangeMetadataRecord(
uint64_t range_version,
uint64_t version,
uint32_t segment_cnt,
int32_t range_size,
RangeMetadataAccumulator &accumulator)
{
// Compute kv_table_name and kv_partition_id
Expand All @@ -1563,8 +1595,8 @@ void DataStoreServiceClient::EnqueueRangeMetadataRecord(
// Encode key and value
std::string key_str =
EncodeRangeKey(catalog_factory, table_name, range_start_key);
std::string rec_str =
EncodeRangeValue(partition_id, range_version, version, segment_cnt);
std::string rec_str = EncodeRangeValue(
partition_id, range_version, version, segment_cnt, range_size);

// Get or create entry in accumulator
auto key = std::make_pair(kv_table_name, kv_partition_id);
Expand Down Expand Up @@ -1732,6 +1764,7 @@ bool DataStoreServiceClient::UpdateRangeSlices(
req.range_slices_,
req.partition_id_);
uint32_t segment_cnt = slice_plan.segment_cnt;
int32_t range_size = slice_plan.range_size;
int32_t kv_partition_id =
KvPartitionIdOfRangeSlices(*req.table_name_, req.partition_id_);
auto iter = slice_plans.find(kv_partition_id);
Expand All @@ -1756,6 +1789,7 @@ bool DataStoreServiceClient::UpdateRangeSlices(
req.range_version_,
req.ckpt_ts_,
segment_cnt,
range_size,
meta_acc);
}

Expand Down Expand Up @@ -1957,6 +1991,7 @@ bool DataStoreServiceClient::UpdateRangeSlices(
range_version,
version,
segment_cnt,
slice_plans[0].range_size,
meta_acc);

SyncConcurrentRequest *meta_sync_concurrent =
Expand Down Expand Up @@ -2048,6 +2083,7 @@ bool DataStoreServiceClient::UpsertRanges(
auto slice_plan = PrepareRangeSliceBatches(
table_name, version, range.slices_, range.partition_id_);
uint32_t segment_cnt = slice_plan.segment_cnt;
int32_t range_size = slice_plan.range_size;

int32_t kv_partition_id =
KvPartitionIdOfRangeSlices(table_name, range.partition_id_);
Expand All @@ -2071,6 +2107,7 @@ bool DataStoreServiceClient::UpsertRanges(
version, // range_version (using version for now)
version,
segment_cnt,
range_size,
meta_acc);
}

Expand Down Expand Up @@ -4651,7 +4688,8 @@ bool DataStoreServiceClient::InitTableRanges(

std::string key_str =
EncodeRangeKey(catalog_factory, table_name, *neg_inf_key);
std::string rec_str = EncodeRangeValue(init_range_id, version, version, 0);
std::string rec_str =
EncodeRangeValue(init_range_id, version, version, 0, 0);

keys.emplace_back(std::string_view(key_str.data(), key_str.size()));
records.emplace_back(std::string_view(rec_str.data(), rec_str.size()));
Expand Down
14 changes: 13 additions & 1 deletion store_handler/data_store_service_client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ struct RangeSliceBatchPlan
std::vector<std::string> segment_keys; // Owned string buffers
std::vector<std::string> segment_records; // Owned string buffers
size_t version;
int32_t range_size{0};

// Clear method for reuse
void Clear()
Expand All @@ -74,6 +75,7 @@ struct RangeSliceBatchPlan
segment_keys.clear();
segment_records.clear();
version = 0;
range_size = 0;
}
};

Expand Down Expand Up @@ -271,6 +273,9 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler

void FetchRangeSlices(txservice::FetchRangeSlicesReq *fetch_cc) override;

void FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc) override;

bool DeleteOutOfRangeData(
const txservice::TableName &table_name,
int32_t partition_id,
Expand Down Expand Up @@ -339,7 +344,8 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler
std::string EncodeRangeValue(int32_t range_id,
uint64_t range_version,
uint64_t version,
uint32_t segment_cnt);
uint32_t segment_cnt,
int32_t range_size);
std::string EncodeRangeSliceKey(const txservice::TableName &table_name,
int32_t range_id,
uint32_t segment_id);
Expand Down Expand Up @@ -642,6 +648,7 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler
uint64_t range_version,
uint64_t version,
uint32_t segment_cnt,
int32_t range_size,
RangeMetadataAccumulator &accumulator);

void DispatchRangeMetadataBatches(
Expand Down Expand Up @@ -922,6 +929,11 @@ class DataStoreServiceClient : public txservice::store::DataStoreHandler
::google::protobuf::Closure *closure,
DataStoreServiceClient &client,
const remote::CommonResult &result);

friend void FetchRangeSizeCallback(void *data,
::google::protobuf::Closure *closure,
DataStoreServiceClient &client,
const remote::CommonResult &result);
};

struct UpsertTableData
Expand Down
49 changes: 45 additions & 4 deletions store_handler/data_store_service_client_closure.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,9 @@ void FetchTableRangesCallback(void *data,
for (uint32_t i = 0; i < items_size; i++)
{
scan_next_closure->GetItem(i, key, value, ts, ttl);
assert(value.size() == (sizeof(int32_t) + sizeof(uint64_t) +
sizeof(uint64_t) + sizeof(uint32_t)));
assert(value.size() ==
(sizeof(int32_t) + sizeof(uint64_t) + sizeof(uint64_t) +
sizeof(uint32_t) + sizeof(int32_t)));
const char *buf = value.data();
int32_t partition_id = *(reinterpret_cast<const int32_t *>(buf));
buf += sizeof(partition_id);
Expand Down Expand Up @@ -926,6 +927,45 @@ void FetchTableRangesCallback(void *data,
}
}

void FetchRangeSizeCallback(void *data,
::google::protobuf::Closure *closure,
DataStoreServiceClient &client,
const remote::CommonResult &result)
{
txservice::FetchTableRangeSizeCc *fetch_range_size_cc =
static_cast<txservice::FetchTableRangeSizeCc *>(data);

if (result.error_code() == remote::DataStoreError::KEY_NOT_FOUND)
{
fetch_range_size_cc->store_range_size_ = 0;
fetch_range_size_cc->SetFinish(
static_cast<uint32_t>(txservice::CcErrorCode::NO_ERROR));
}
else if (result.error_code() != remote::DataStoreError::NO_ERROR)
{
LOG(ERROR) << "Fetch range size failed with error code: "
<< result.error_code();
fetch_range_size_cc->SetFinish(
static_cast<uint32_t>(txservice::CcErrorCode::DATA_STORE_ERR));
}
else
{
ReadClosure *read_closure = static_cast<ReadClosure *>(closure);
std::string_view read_val = read_closure->Value();
assert(read_closure->TableName() == kv_range_table_name);
assert(read_val.size() ==
(sizeof(int32_t) + sizeof(uint64_t) + sizeof(uint64_t) +
sizeof(uint32_t) + sizeof(int32_t)));
const char *buf = read_val.data();
buf += read_val.size() - sizeof(int32_t);
fetch_range_size_cc->store_range_size_ =
*reinterpret_cast<const int32_t *>(buf);
Comment on lines +956 to +962
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "data_store_service_client_closure.cpp" -type f

Repository: eloqdata/tx_service

Length of output: 116

🏁 Script executed:

wc -l ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 121

🏁 Script executed:

sed -n '950,970p' ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 866

🏁 Script executed:

sed -n '930,975p' ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 2006

🏁 Script executed:

# Check if there's any build configuration that defines assert behavior
find . -name "CMakeLists.txt" -o -name "*.cmake" | head -5

Repository: eloqdata/tx_service

Length of output: 188

🏁 Script executed:

# Check the CMakeLists.txt for build configuration, especially Debug vs Release handling
head -100 ./CMakeLists.txt | grep -A 5 -B 5 "NDEBUG\|assert\|Debug\|Release"

Repository: eloqdata/tx_service

Length of output: 45

🏁 Script executed:

grep -i "ndebug\|cmake_build_type" ./CMakeLists.txt | head -20

Repository: eloqdata/tx_service

Length of output: 45

🏁 Script executed:

# Search for similar patterns in the file - other uses of assert before reinterpret_cast
grep -n "assert.*size()" ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 333

🏁 Script executed:

# Check the other similar patterns to see if they have the same issue
sed -n '790,810p' ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 1020

🏁 Script executed:

# Check the third similar pattern at line 1009
sed -n '1005,1025p' ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 1048

🏁 Script executed:

# Let's understand where read_closure->Value() comes from - check for ReadClosure definition
grep -n "class ReadClosure\|struct ReadClosure" ./store_handler/*.cpp ./store_handler/*.h

Repository: eloqdata/tx_service

Length of output: 334

🏁 Script executed:

sed -n '600,700p' ./store_handler/data_store_service_client_closure.h

Repository: eloqdata/tx_service

Length of output: 3387

🏁 Script executed:

sed -n '700,800p' ./store_handler/data_store_service_client_closure.h

Repository: eloqdata/tx_service

Length of output: 2563

🏁 Script executed:

sed -n '800,850p' ./store_handler/data_store_service_client_closure.h

Repository: eloqdata/tx_service

Length of output: 911

🏁 Script executed:

# Check if there are any other safety checks or validations before the problematic code
sed -n '930,965p' ./store_handler/data_store_service_client_closure.cpp

Repository: eloqdata/tx_service

Length of output: 1648

Replace assert() with runtime validation before decoding store_range_size_.

The size check at line 956 relies solely on assert(), which is disabled in Release builds. Malformed payloads will reach the pointer arithmetic and reinterpret_cast operations unchecked, causing buffer overread from untrusted data. Add a runtime check with proper error handling:

Suggested fix 
-        assert(read_val.size() ==
-               (sizeof(int32_t) + sizeof(uint64_t) + sizeof(uint64_t) +
-                sizeof(uint32_t) + sizeof(int32_t)));
-        const char *buf = read_val.data();
-        buf += read_val.size() - sizeof(int32_t);
-        fetch_range_size_cc->store_range_size_ =
-            *reinterpret_cast<const int32_t *>(buf);
+        constexpr size_t kExpectedRangeMetaSize =
+            sizeof(int32_t) + sizeof(uint64_t) + sizeof(uint64_t) +
+            sizeof(uint32_t) + sizeof(int32_t);
+        if (read_val.size() != kExpectedRangeMetaSize)
+        {
+            LOG(ERROR) << "Fetch range size decode failed: unexpected value size="
+                       << read_val.size();
+            fetch_range_size_cc->SetFinish(
+                static_cast<uint32_t>(txservice::CcErrorCode::DATA_STORE_ERR));
+            return;
+        }
+        const char *buf = read_val.data() + read_val.size() - sizeof(int32_t);
+        fetch_range_size_cc->store_range_size_ =
+            *reinterpret_cast<const int32_t *>(buf);

Note: Similar patterns exist at lines 796 and 1009 that may need the same treatment.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/data_store_service_client_closure.cpp` around lines 956 - 962,
The assert on read_val.size() must be replaced with a runtime size validation
before doing buf arithmetic and the reinterpret_cast: check that read_val.size()
==
(sizeof(int32_t)+sizeof(uint64_t)+sizeof(uint64_t)+sizeof(uint32_t)+sizeof(int32_t))
and if not, handle the error path (e.g., set an error status/return an error
code, log and abort parsing) instead of proceeding; then compute const char *buf
= read_val.data() + read_val.size() - sizeof(int32_t) and safely decode
fetch_range_size_cc->store_range_size_ = *reinterpret_cast<const int32_t
*>(buf). Apply the same runtime validation/failure handling to the similar
patterns around the uses at the other locations (the checks near the code that
sets fetch_range_size_cc->store_range_size_ and the spots mentioned at the other
offsets).


fetch_range_size_cc->SetFinish(
static_cast<uint32_t>(txservice::CcErrorCode::NO_ERROR));
}
}

void FetchRangeSlicesCallback(void *data,
::google::protobuf::Closure *closure,
DataStoreServiceClient &client,
Expand Down Expand Up @@ -966,8 +1006,9 @@ void FetchRangeSlicesCallback(void *data,
else
{
assert(read_closure->TableName() == kv_range_table_name);
assert(read_val.size() == (sizeof(int32_t) + sizeof(uint64_t) +
sizeof(uint64_t) + sizeof(uint32_t)));
assert(read_val.size() ==
(sizeof(int32_t) + sizeof(uint64_t) + sizeof(uint64_t) +
sizeof(uint32_t) + sizeof(int32_t)));
const char *buf = read_val.data();
int32_t range_partition_id =
*(reinterpret_cast<const int32_t *>(buf));
Expand Down
8 changes: 8 additions & 0 deletions store_handler/data_store_service_client_closure.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3102,6 +3102,14 @@ void FetchTableRangesCallback(void *data,
DataStoreServiceClient &client,
const remote::CommonResult &result);

/**
* Callback for fetching range size from table_ranges.
*/
void FetchRangeSizeCallback(void *data,
::google::protobuf::Closure *closure,
DataStoreServiceClient &client,
const remote::CommonResult &result);

/**
* Callback for fetching range slices.
*
Expand Down
6 changes: 6 additions & 0 deletions store_handler/dynamo_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2534,6 +2534,12 @@ void EloqDS::DynamoHandler::FetchRangeSlices(FetchRangeSlicesReq *fetch_cc)
assert(false);
}

void EloqDS::DynamoHandler::FetchTableRangeSize(FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "DynamoHandler::FetchTableRangeSize not implemented";
assert(false);
}
Comment on lines +2537 to +2541
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not leave FetchTableRangeSizeCc unfinished on unsupported path.

Line 2540 aborts in debug and becomes a silent early return in release (assert removed), so fetch_cc may never be completed. Return a datastore error via fetch_cc instead of asserting.

Proposed fix 
 void EloqDS::DynamoHandler::FetchTableRangeSize(FetchTableRangeSizeCc *fetch_cc)
 {
     LOG(ERROR) << "DynamoHandler::FetchTableRangeSize not implemented";
-    assert(false);
+    fetch_cc->SetFinish(static_cast<int>(txservice::CcErrorCode::DATA_STORE_ERR));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void EloqDS::DynamoHandler::FetchTableRangeSize(FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "DynamoHandler::FetchTableRangeSize not implemented";
assert(false);
}
void EloqDS::DynamoHandler::FetchTableRangeSize(FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "DynamoHandler::FetchTableRangeSize not implemented";
fetch_cc->SetFinish(static_cast<int>(txservice::CcErrorCode::DATA_STORE_ERR));
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/dynamo_handler.cpp` around lines 2537 - 2541, The function
EloqDS::DynamoHandler::FetchTableRangeSize currently aborts via assert and
leaves fetch_cc uncompleted; instead, set an appropriate datastore error on
fetch_cc and invoke its completion path so the caller is notified (do not
assert). Locate EloqDS::DynamoHandler::FetchTableRangeSize and replace the
assert with code that populates fetch_cc with an error status/message indicating
"unsupported on this path" (matching the error pattern used by other handlers)
and then call the same completion method used elsewhere for these CC objects
(the fetch_cc completion/send/done method used by other handlers) so fetch_cc is
always completed. Ensure you reference fetch_cc and the function name
EloqDS::DynamoHandler::FetchTableRangeSize when making the change.


void EloqDS::DynamoHandler::OnFetchRangeSlices(
const Aws::DynamoDB::DynamoDBClient *client,
const Aws::DynamoDB::Model::GetItemRequest &request,
Expand Down
1 change: 1 addition & 0 deletions store_handler/dynamo_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ class DynamoHandler : public txservice::store::DataStoreHandler
//-- range partition
void FetchTableRanges(FetchTableRangesCc *fetch_cc) override;
void FetchRangeSlices(FetchRangeSlicesReq *fetch_cc) override;
void FetchTableRangeSize(FetchTableRangeSizeCc *fetch_cc) override;

bool DeleteOutOfRangeData(
const txservice::TableName &table_name,
Expand Down
7 changes: 7 additions & 0 deletions store_handler/rocksdb_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,13 @@ void RocksDBHandler::FetchRangeSlices(txservice::FetchRangeSlicesReq *fetch_cc)
assert(false);
}

void RocksDBHandler::FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc)
{
LOG(ERROR) << "RocksDBHandler::FetchTableRangeSize not implemented";
assert(false);
}
Comment on lines +1113 to +1118
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Avoid process abort in FetchTableRangeSize runtime path.

Line 1117 hard-aborts (assert(false)). Since this API is now wired into range-size tracking, invoking it on RocksDB will crash the node. Return a datastore error through fetch_cc and complete the request instead of asserting.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/rocksdb_handler.cpp` around lines 1113 - 1118, Replace the
assert-based abort in RocksDBHandler::FetchTableRangeSize so it returns a proper
datastore error instead of crashing: detect that the operation is unsupported on
RocksDB, populate an appropriate error/status on the provided
txservice::FetchTableRangeSizeCc* fetch_cc (e.g., set a
datastore-not-implemented or internal error with a clear message), and then
complete the request by invoking the fetch_cc completion/reply method so the
caller receives an error response rather than triggering assert(false).


bool DeleteOutOfRangeDataInternal(std::string delete_from_partition_sql,
int32_t partition_id,
const txservice::TxKey *start_k)
Expand Down
3 changes: 3 additions & 0 deletions store_handler/rocksdb_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ class RocksDBHandler : public txservice::store::DataStoreHandler

void FetchRangeSlices(txservice::FetchRangeSlicesReq *fetch_cc) override;

void FetchTableRangeSize(
txservice::FetchTableRangeSizeCc *fetch_cc) override;

Comment on lines +349 to +351
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n -A 15 'RocksDBHandler::FetchTableRangeSize' --type=cpp

Repository: eloqdata/tx_service

Length of output: 1396

🏁 Script executed:

rg -n -A 15 'BigTableHandler::FetchTableRangeSize' --type=cpp

Repository: eloqdata/tx_service

Length of output: 1492

Use proper error handling instead of assert(false) for unsupported operations.

The current stub implementation logs an error but crashes via assert(false), which is unsafe in production (becomes a no-op in release builds). Instead, set appropriate error codes or status on fetch_cc so callers can detect and respond to the unsupported operation gracefully. Note that BigTableHandler has the same issue and should be fixed similarly.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@store_handler/rocksdb_handler.h` around lines 349 - 351, The
FetchTableRangeSize override currently asserts(false) on unsupported ops;
replace that crash with proper error handling by setting an error/status on the
txservice::FetchTableRangeSizeCc* fetch_cc (e.g., set an error code and
descriptive error message indicating "operation not supported" or similar) and
return gracefully; do the same fix for the BigTableHandler equivalent to ensure
callers can check fetch_cc for failures instead of relying on assertions. Ensure
you use the fetch_cc API (status/error fields or setter methods present on
txservice::FetchTableRangeSizeCc) to communicate the failure and do not call
assert or abort.

bool DeleteOutOfRangeDataInternal(std::string delete_from_partition_sql,
int32_t partition_id,
const txservice::TxKey *start_k);
Expand Down
3 changes: 2 additions & 1 deletion tx_service/include/cc/cc_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ class CcHandler
const TxRecord *record,
OperationType operation_type,
uint32_t key_shard_code,
CcHandlerResult<PostProcessResult> &hres) = 0;
CcHandlerResult<PostProcessResult> &hres,
uint8_t range_size_flags = 0x10) = 0;

/**
* @briefPost-processes a read/scan key. Post-processing clears the read
Expand Down
Loading