Take string view arguments in cache processor

[hnakamur]

No description provided.

[cmcfarlen]

I don't think this PR changes layout of any disk-stored structs.

[hnakamur]

Yes, I agree with you. This PR changes the field of struct CacheVC but I suppose it is not stored on disk.

I found this branch has conflicts from with changes from Cleanup: Remove unused args from open_read and open_write by masaori335 · Pull Request #12181 · apache/trafficserver.
I merged master to the copy of this branch, resolved conflicts and pushed it to https://github.com/hnakamur/trafficserver/tree/take_string_view_arguments_in_cache_processor2.

Can I foward this branch to f7e77f9?

[masaori335]

We're going to do "squash & merge" to merge this PR, so forwarding this branch to f7e77f9 should be fine. Please push the change, let's see what diff shows.

[hnakamur]

Oops, I found I created the https://github.com/hnakamur/trafficserver/tree/take_string_view_arguments_in_cache_processor2 from the wrong branch.
I'm going to fix it later today.

[masaori335]

@cmcfarlen This is part of "Region C". Can we change this? IIRC, I broke cache compatibility by changing type in this region.

[masaori335]

@bneradt do you remember what you faced with time_t pin_in_cache?
#11013

[bneradt]

Yes, you are correct about the pin_in_cache issue. Coverity reports type issues on that pin_in_cache. Two different commits went in to change its type to address the coverity warning (not just your commit, @masaori335 ). When that got changed to a 64 bit type rather than a 32 bit one, it resulted in cache incompatibility. Upgrading through that change would result in many warnings about cache inconsistency.

@masaori335 is correct to be leary of this change. Maybe it would be fine? What are the underlying types of a string_view - if they are char* and int anyway, maybe it won't cause issues? But we would want to upgrade a box with cached items to test this and verify there are not cache inconsistency warnings.

[masaori335]

Let me check this doesn't break cache compatibility.

[hnakamur]

@masaori335

Let me check this doesn't break cache compatibility.

Thank you. I'll revert the change in struct CacheVC if it causes cache incompatibility.

We're going to do "squash & merge" to merge this PR, so forwarding this branch to f7e77f9 should be fine. Please push the change, let's see what diff shows.

The merge commit f7e77f9 contains the changes in other branches which were merged to master. However the diff betwen master and f7e77f9 master...hnakamur:f7e77f9 contains only changes in this branch.

So I think it should be OK to forward this branch to f7e77f9.