moonchen commented on PR #13380: URL: https://github.com/apache/trafficserver/pull/13380#issuecomment-5109845464
Read through this and ran the tests locally. The bug reproduces as described: `http_copy_hdr` at `CacheVC.cc:403` is computed from `cache_config_ram_cache_compress` and the doc properties with no reference to the algorithm, and `RecordsConfig.cc:863` defaults `ram_cache.algorithm` to 1, so the default policy was the one ignoring the flag. I checked a couple of other cases: Tested on a 5000 byte object to exercise the size rounding in `iobuffer_size_to_index()`. The tests still pass. Worth using a non-power-of-two payload anyway, so the tests do not depend on 8192 landing exactly on a size index. Checked the `ram_cache_bytes` gauge across the resident refresh: N objects put with `copy=false`, re-put with `copy=true`, then the cache overflowed to force eviction. The gauge rises on the puts and does not go negative through eviction, for all three policies. That path is the only place the fix does arithmetic rather than just swapping the buffer, so it is worth a permanent test. I also recommend an S3-FIFO ghost admit test. The copy handling is inherited -- the ghost is removed and the fresh entry goes through the same insert site -- but the readmit lands in `SEG_MAIN` and accounts against `_m_bytes`, and with `copy` set the charged size is `len` rather than `data->block_size()`. That combination is not covered. Happy to approve once it comes out of draft. -- 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]
