lucasfang commented on code in PR #286:
URL: https://github.com/apache/paimon-cpp/pull/286#discussion_r3975047532
##########
src/paimon/common/reader/prefetch_file_batch_reader_impl.h:
##########
@@ -123,10 +125,19 @@ class PrefetchFileBatchReaderImpl : public
PrefetchFileBatchReader {
uint32_t prefetch_queue_capacity, bool
enable_adaptive_prefetch_strategy,
const std::shared_ptr<Executor>& executor, const
std::shared_ptr<ReadAheadCache>& cache,
const std::shared_ptr<PrefetchIoMetricsState>& io_metrics,
- const std::shared_ptr<arrow::MemoryPool>& arrow_pool);
+ const std::shared_ptr<arrow::MemoryPool>& arrow_pool, WarmupLevel
warmup_level);
Status CleanUp();
void Workloop();
+ /// Starts the background prefetch thread if it is not running yet. The
first read does this
+ /// itself; Warmup() is the same call made earlier, so the two must not
diverge.
+ void EnsureBackgroundThread();
+ /// Initializes and warms the read-ahead cache at most once per read-range
generation, using the
+ /// first reader's PreBufferRange(). Shared by Workloop() (DECODED) and
Warmup() (RAW), so the
+ /// two never Init the cache twice. A no-op when there is no cache. Errors
are recorded via
+ /// SetReadStatus() rather than returned: a warmup hint for a file that
may never be read must
+ /// not fail an in-flight read.
+ void WarmCacheOnce();
Review Comment:
You're right, exchange(true) only elects an initializer and never makes the
loser wait — the race claim was wrong.Went with the first option: relying on
the documented non-concurrent contract. Warmup() must be called from the thread
that reads the reader, so a RAW warmup runs WarmCacheOnce() on that thread
before the same thread starts the background thread running Workloop(). Thread
creation gives happens-before, so the two calls are ordered rather than
concurrent, and the loser of the exchange can never proceed on a cache that
another thread is still initializing. Comments in both the header and the
implementation now say exactly that, and state that the flag's only job is to
avoid a second non-idempotent Init(). No logic change.
--
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]
