liaoxin01 commented on code in PR #68310:
URL: https://github.com/apache/doris/pull/68310#discussion_r4060026767
##########
be/test/io/cache/cached_remote_file_reader_test.cpp:
##########
@@ -178,6 +178,26 @@ class AsyncCachedRemoteFileReaderTest : public
BlockFileCacheTest {
} // namespace
+TEST_F(AsyncCachedRemoteFileReaderTest, sync_write_path_preserves_tablet_id) {
+ create_cache("cached_remote_reader_sync_write_tablet_id");
+ auto reader = create_reader(open_remote_file());
+
+ std::string result(64_kb, '\0');
+ FileCacheStatistics stats;
+ IOContext context;
+ context.file_cache_stats = &stats;
+ context.is_warmup = true;
+ size_t bytes_read = 0;
+ ASSERT_TRUE(
+ reader->read_at(0, Slice(result.data(), result.size()),
&bytes_read, &context).ok());
+ EXPECT_EQ(bytes_read, result.size());
+ EXPECT_EQ(result, std::string(result.size(), '0'));
+
+ const auto blocks = cache()->get_blocks_by_key(reader->_cache_hash);
+ ASSERT_EQ(blocks.size(), 1);
+ EXPECT_EQ(blocks.begin()->second->tablet_id(), 10086);
Review Comment:
This is a valid pre-existing lifecycle gap, but it is not introduced by this
PR. Async cache writes already persisted positive tablet IDs before this
change, while startup and direct restoration have always called add_cell
directly without register_tablet_id. This PR only fixes newly populated
synchronous blocks and does not modify or regress restoration. Also, FS storage
starts its background load before BlockFileCache constructs the TTL manager, so
restart registration needs a separate lifecycle change and persistence/restart
test. It should be tracked in a dedicated follow-up rather than expanding this
focused fix.
##########
be/src/io/cache/cached_remote_file_reader.cpp:
##########
@@ -1078,6 +1078,7 @@ Status
CachedRemoteFileReader::_read_from_indirect_cache(size_t offset, Slice re
s_align_size(offset + already_read, bytes_req - already_read,
size());
CacheContext cache_context(io_ctx);
cache_context.stats = &stats;
+ cache_context.tablet_id = _tablet_id;
Review Comment:
Addressed in a9b77a53979. CachedRemoteFileReader now normalizes external
readers to the no-tablet sentinel 0 at construction, so both synchronous and
asynchronous cache paths persist the same metadata namespace. TTL registration
also defensively accepts only positive tablet IDs. The added unit test
exercises both write modes and verifies that external blocks retain tablet ID 0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
