Lost fragments table when update object in Cache

[taaotao007]

when set 404 response cached, first time got 404 response, then next request got 200 response,but fragment offset did not changed, this cause range request data error.

[randall]

[approve ci]

[scw00]

Sometime we try to update the header only fragment that means we won't write any fragment in it. So the total_len is always 0 in this situation. And we will lost the fragments info forever if we don't copy the fragments table from old info.

[taaotao007]

There is situation that first response code is 404 and next response code is 200,so the total_len has changed to non zero.In this situation,this cause data read error.

[scw00]

But it affect the normal requests.

[taaotao007]

You can test this situation.

[zwoop]

Besides fixing the build problems, can you also please make a more descriptive (but < 50 character) Subject line for this PR? Summarizing the more descriptive text in the commit.

[zwoop]

[approve ci].

[shinrich]

[approve ci]

[scw00]

I won't approve this pr because it break the header update only case. As I comment above, some header update object will lost the fragments table forever.

For your case. My question is:

• How did you cache HTTP with negative code ?
• How did you update the negative HTTP with 200 HTTP ?
• How to reproduce that ?

[taaotao007]

I won't approve this pr because it break the header update only case. As I comment above, some header update object will lost the fragments table forever.

For your case. My question is:

• How did you cache HTTP with negative code ?
• How did you update the negative HTTP with 200 HTTP ?
• How to reproduce that ?

1 jush set proxy.config.http.negative_caching_enabled 1
2 when client reqests, may be the resource just producing,at this time responsed with a 404 status,after the data is produced, the client requests get a 200 status.

[scw00]

Thanks.

So you mean the 404 is stale and ats goes back to the OS. And then OS return 200 to ats. That make sense, will test it in this weekend.

[taaotao007]

Thanks.

So you mean the 404 is stale and ats goes back to the OS. And then OS return 200 to ats. That make sense, will test it in this weekend.

yes

[scw00]

Yes I see.

In fact, it is not only negative cache issue but also the whole cache update cases I think . When someone want to update an object with response data (not header only cases), it always copy fragments table from old info.

trafficserver/iocore/cache/CacheWrite.cc

Lines 108 to 112 in 88b5a13

	if (alternate_index >= 0) {
	alternate.copy_frag_offsets_from(write_vector->get(alternate_index));
	}
	alternate_index = write_vector->insert(&alternate, alternate_index);
	}

How about changing the L108 to if (alternate_index >= 0 && ((total_len == 0 && alternate.get_frag_offset_count() == 0) && !(f.allow_empty_doc && this->vio.nbytes == 0)))

like that

+      if (alternate_index >= 0 && ((total_len == 0 && alternate.get_frag_offset_count() == 0) && !(f.allow_empty_doc && this->vio.nbytes == 0))) {
+         alternate.copy_frag_offsets_from(write_vector->get(alternate_index));
+       }

       alternate_index = write_vector->insert(&alternate, alternate_index);

the total_len is checking whether the user has written data here. So if the user write something the frag_counts should not be 0. And we don't need to copy anything. But if not, that was the header update only case . And we should keep the fragments table from old info. Also be care for the empty doc case. The empty doc should copy nothing .

[scw00]

@taaotao007 Do you want to update your PR ?

[scw00]

Thanks to your report. open an issue #5694 to trace it . Please feel free to reopen it.