Copilot commented on code in PR #13215:
URL: https://github.com/apache/trafficserver/pull/13215#discussion_r3325538350
##########
plugins/slice/util.cc:
##########
@@ -67,7 +67,14 @@ schedule_prefetch(Data *const data)
nextblocknum = data->m_blocknum + data->m_config->m_prefetchcount;
}
- for (int i = nextblocknum; i <= data->m_blocknum +
data->m_config->m_prefetchcount; i++) {
+ int const lastblocknum = data->m_blocknum + data->m_config->m_prefetchcount;
+
+ // Skip blocks already scheduled this request; re-issuing them races the
inline reads.
+ if (nextblocknum <= data->m_prefetch_hwm) {
+ nextblocknum = data->m_prefetch_hwm + 1;
Review Comment:
`m_blocknum` is `int64_t`, but `lastblocknum` / loop index are `int`. For
large byte ranges (large block numbers), this can truncate/overflow and cause
incorrect prefetch scheduling and an incorrect high-water mark update. Using
`int64_t` for the computations and only narrowing when calling
`BgBlockFetch::schedule()` keeps the logic correct and makes the narrowing
explicit.
##########
plugins/slice/Data.h:
##########
@@ -94,6 +94,8 @@ struct Data {
bool m_prefetchable{false};
+ int m_prefetch_hwm{-1}; // highest block this request has scheduled for
prefetch
Review Comment:
`m_prefetch_hwm` tracks a block number, but it is stored as `int` while the
rest of the block math (`m_blocknum`, `Range::*BlockFor`) is `int64_t`. This
can truncate/overflow for large ranges and break the new high-water-mark logic.
--
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]
