github-actions[bot] commented on code in PR #68847:
URL: https://github.com/apache/doris/pull/68847#discussion_r4236783493
##########
regression-test/suites/external_table_p0/tvf/test_http_tvf.groovy:
##########
@@ -317,6 +324,27 @@ suite("test_http_tvf", "p0") {
) order by id;
"""
+ // The same URL serving new content of the same length: an HTTP response
has no modification
+ // time for the file cache key, so the second query must not read blocks
cached by the first.
+ sql """ set enable_file_cache = true """
Review Comment:
[P2] Enable BE file caching for the mutable HTTP regression. This `SET`
changes only the query option; `FileFactory::get_reader_options` also requires
BE `config::enable_file_cache`, whose default is false and which the
p0/external BE configurations do not set. In those runs both queries bypassed
the cache even before this PR, so the new expected rows cannot detect a
stale-cache regression. Run this case with a cache-enabled BE fixture (or an
equivalent read-through unit test) and verify the second version's bytes.
##########
be/src/io/file_factory.cpp:
##########
@@ -248,10 +262,12 @@ Result<io::FileReaderSPtr>
FileFactory::_create_file_reader_internal(
}
RETURN_IF_ERROR_RESULT(ExecEnv::GetInstance()->hdfs_mgr()->get_or_create_fs(
system_properties.hdfs_params, *fs_name, &handler));
+ // The reader's path drops the name node, so the cache key takes it
from here.
+ auto options = with_storage_in_cache_key(reader_options, *fs_name);
Review Comment:
[P1] Include the resolved HDFS configuration in this cache key. Two catalogs
can use the same `hdfs://ns/path` while mapping `ns` to different NameNodes
through `dfs.namenode.rpc-address.ns.*`; `HdfsMgr` creates distinct clients for
their different `hdfs_conf`, but `*fs_name`, the reader's stripped path, and an
equal mtime produce the same cache hash. A scan of the second cluster can
therefore return the first cluster's cached blocks. Include a stable
fingerprint of the effective HDFS connection in the key and cover this
same-name/different-NameNode case.
--
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]
