Copilot commented on code in PR #13328:
URL: https://github.com/apache/trafficserver/pull/13328#discussion_r3687452304
##########
src/iocore/cache/StripeSM.cc:
##########
@@ -1326,9 +1347,26 @@ StripeSM::shutdown(EThread *shutdown_thread)
SCOPED_MUTEX_LOCK(lock, this->mutex, shutdown_thread);
if (DISK_BAD(this->disk)) {
- Dbg(dbg_ctl_cache_dir_sync, "Dir %s: ignoring -- bad disk",
this->hash_text.get());
+ // Bad disk: invalidate the shm copy so the next start recovers from disk
+ // (mirrors the flush-failure branch below).
+ Dbg(dbg_ctl_cache_dir_sync, "Dir %s: bad disk -- invalidating shm copy for
disk recovery", this->hash_text.get());
+ CacheShm::invalidate_stripe_directory(this->directory.raw_dir);
return;
}
+
+ bool shm_backed = CacheShm::is_shm_pointer(this->directory.raw_dir);
+
+ // In-flight AIO write: aggWriteDone advances write_pos again after we drop
this mutex, so
+ // the shm header is not final to trust verbatim. Invalidate it and fall
through to the
+ // on-disk write so the next start recovers from a fresh snapshot (short
recover_data()
+ // scan) instead of a stale periodic-sync copy -- the same path a non-shm
stripe takes,
+ // and recover_data() rescans the tail so the imprecise write_pos is
reconciled.
+ if (shm_backed && this->is_io_in_progress()) {
+ Dbg(dbg_ctl_cache_dir_sync, "Dir %s: AIO write in flight -- invalidating
shm copy, syncing dir to disk", this->hash_text.get());
+ CacheShm::invalidate_stripe_directory(this->directory.raw_dir);
+ shm_backed = false;
+ }
Review Comment:
In the `is_io_in_progress()` shutdown path,
`CacheShm::invalidate_stripe_directory(this->directory.raw_dir)` zeroes the
stripe header magic in-place. Because the on-disk directory write later uses
`this->directory.raw_dir` as the pwrite buffer, this path can end up writing a
directory with an invalid magic to disk (after `shm_backed` is forced to
`false`), which can break/extend recovery on the next start and defeats the
intent of writing a “fresh snapshot”.
Consider restructuring so the shm invalidation does not mutate the buffer
used for the disk directory write (e.g., write the on-disk directory from a
temporary copy taken before invalidation, or use a separate per-stripe
invalidation marker that doesn’t affect the disk-write buffer).
--
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]
