On 03/01/2012, at 11:15 AM, Brian Geffon wrote:
> So you're right, now that I have the full code in front of me...I understand
> what you're saying, my previous email was wrong, but I don't see where the
> discrepancy could be coming from because a loop over
> TSIOBufferBlockReadStart() over all blocks appears to do the exact same thing
> as TSIOBufferReaderAvail().
>
> Here is what happens when you call TSIOBufferReaderAvail():
>
> TS_INLINE int64_t
> IOBufferReader::read_avail()
> {
> int64_t t = 0;
> IOBufferBlock *b = block;
>
> while (b) {
> t += b->read_avail();
> b = b->next;
> }
> t -= start_offset;
> if (size_limit != INT64_MAX && t > size_limit)
> t = size_limit;
> return t;
> }
>
> It's summing up the sizes of all the blocks and backing out the start_offset,
> now if we examine the code of TSIOBufferBlockReadStart(), I added two small
> comments below to the function
>
> const char *
> TSIOBufferBlockReadStart(TSIOBufferBlock blockp, TSIOBufferReader readerp,
> int64_t *avail)
> {
> sdk_assert(sdk_sanity_check_iocore_structure(blockp) == TS_SUCCESS);
> sdk_assert(sdk_sanity_check_iocore_structure(readerp) == TS_SUCCESS);
>
> IOBufferBlock *blk = (IOBufferBlock *)blockp;
> IOBufferReader *reader = (IOBufferReader *)readerp;
> char *p;
>
> p = blk->start();
> if (avail)
> *avail = blk->read_avail(); // avail is set to the size of this block
>
> if (blk == reader->block) {
> // if this is the first block, then advance the pointer and back out the
> start offset
> p += reader->start_offset;
> if (avail) {
> *avail -= reader->start_offset;
> if (*avail < 0) {
> *avail = 0;
> }
> }
> }
>
> return (const char *)p;
> }
>
> So now looking again, the only possible issue could be that you're using
> TSIOBufferStart() instead of TSIOBufferReaderStart(), can you try using
> TSIOBufferReaderStart() to get the first block instead of using
> TSIOBufferStart(), because TSIOBufferStart() will try to add new blocks if
> the current block can't be written to, which means that you could potentially
> bypass the first block and get a new empty block.
I switched to TSIOBufferReaderStart() and haven't been able to repro yet ...
very promising!
thanks,
James
>
>
> Brian
>
> ________________________________________
> From: James Peach [jamespe...@me.com]
> Sent: Monday, January 02, 2012 6:47 PM
> To: dev@trafficserver.apache.org
> Subject: Re: inconsistent read IOBuffer results
>
> On 02/01/2012, at 6:13 PM, Brian Geffon wrote:
>
>> My last email was incorrect, it will correctly return the pointer to the
>> memory of the block, but avail will not be touched and you will have no
>> idea how much data could have been read. You actually don't even need to
>> call TSIOBufferBlockReadStart(), TSIOBufferReadAvail() should just be
>> equal to the sum of all of the calls to the TSIOBufferBlockReadAvail().
>
> Yes, that's exactly the invariant that is being violated. I can sometimes end
> up with TSIOBufferReadAvail() claiming there are more bytes than are actually
> available from the buffer.
>
> I'm inside the TSVConnRead() continuation, responding to
> TS_EVENT_VCONN_READ_READY, so I don't *think* that I need to do any
> additional locking. It might be an internal race, but I have slightly hacked
> up my ATS tree, so it's also possible that I caused a stale value to be
> reported by TSIOBufferReadAvail().
>
>> The following is untested but it appears that it should give what you want:
>>
>> block = TSIOBufferStart(buffer);
>> while (block) {
>> count += TSIOBufferBlockReadAvail(block,reader);
>> block = TSIOBufferBlockNext(block);
>> }
>
> That's a more succinct version of the code I posted, right?
>
>>
>> Brian
>>
>>
>>
>> On 1/2/12 6:04 PM, "Brian Geffon" <briangef...@gmail.com> wrote:
>>
>>> I dont think this is a bug, your usage of TSIOBufferBlockReadStart()
>>> is incorrect. The third parameter which you call nbytes is set to the
>>> number of bytes remaining in the buffer block after the read. Since
>>> you're setting it to zero before the call to
>>> TSIOBufferBlockReadStart() it's actually not going to read anything...
>>>
>>> if (avail)
>>> *avail = blk->read_avail();
>>>
>>> So since it's not getting the correct size of the block, and thus will
>>> not reading anything anyway because before it tries to read the block
>>> it does:
>>>
>>> if (avail) {
>>> *avail -= reader->start_offset;
>>> if (*avail < 0) {
>>> *avail = 0;
>>> }
>>> }
>>>
>>> So it's never going to touch avail. So that means nbytes be set to
>>> TSIOBufferBlockReadAvail() before the call
>>>
>>> int64_t nbytes = TSIOBufferBlockReadAvail( ... )
>>> int64_t nbytes_remaining = nbytes;
>>>
>>> and then, nbytes_remaining will be the number of bytes remaining to be
>>> read, which should actually be 0 after the call to
>>> TSIOBufferBlockReadStart(), So the total bytes contained in the block
>>> was :
>>>
>>> ptr = TSIOBufferBlockReadStart(block, reader, &nbytes_remaining);
>>> if(ptr) {
>>> int64_t bytes_in_block = nbytes - nbytes_remaining; // that's how
>>> much we actually were able to read
>>> count += bytes_in_block;
>>> }
>>>
>>> Does that help?
>>>
>>> On Mon, Jan 2, 2012 at 5:45 PM, James Peach <jamespe...@me.com> wrote:
>>>> On 02/01/2012, at 1:18 PM, James Peach wrote:
>>>>
>>>>> On 02/01/2012, at 11:30 AM, Brian Geffon wrote:
>>>>>
>>>>>> I think you might want TSIOBufferBlockReadAvail and not
>>>>>> TSIOBufferReaderAvail.
>>>>>
>>>>> Hmm, so my code is assuming that all the data is in the first buffer
>>>>> block. It sounds like that it not guaranteed and that I ought to be
>>>>> calling TSIOBufferBlockNext() if the first buffer block doesn't have
>>>>> all the data.
>>>>
>>>> After some more testing, I think that this is a bug.
>>>> TSIOBufferReaderAvail() returns 43, but there are subsequently 0 bytes
>>>> available. Iterating the buffer blocks as below confirms this. My
>>>> understanding of TSIOBufferReaderAvail() and the assumption of other
>>>> usages of it that I have seen is that the data in teh buffer blocks
>>>> should all add up to the available count from the buffer reader. I
>>>> haven't seen any indication that TSIOBufferReaderAvail() is advisory.
>>>>
>>>> static size_t
>>>> count_bytes_available(
>>>> TSIOBuffer buffer,
>>>> TSIOBufferReader reader)
>>>> {
>>>> TSIOBufferBlock block;
>>>> size_t count = 0;
>>>>
>>>> block = TSIOBufferStart(buffer);
>>>> while (block) {
>>>> const char * ptr;
>>>> int64_t nbytes = 0;
>>>>
>>>> ptr = TSIOBufferBlockReadStart(block, reader, &nbytes);
>>>> if (ptr && nbytes) {
>>>> count += nbytes;
>>>> }
>>>>
>>>> block = TSIOBufferBlockNext(block);
>>>> }
>>>>
>>>> return count;
>>>> }
>>>>
>>>>>
>>>>> thanks,
>>>>> James
>>>>>
>>>>>>
>>>>>> Brian
>>>>>> ________________________________________
>>>>>> From: James Peach [jamespe...@me.com]
>>>>>> Sent: Saturday, December 31, 2011 10:07 PM
>>>>>> To: dev@trafficserver.apache.org
>>>>>> Subject: inconsistent read IOBuffer results
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> In my proxy code, I have something that looks roughly like this:
>>>>>>
>>>>>> if (TSIOBufferReaderAvail(reader) >= 10) {
>>>>>> blk = TSIOBufferStart(buffer);
>>>>>> ptr = (const uint8_t *)TSIOBufferBlockReadStart(blk,
>>>>>> reader, &nbytes);
>>>>>>
>>>>>> TSReleaseAssert(nbytes >= 10);
>>>>>> }
>>>>>>
>>>>>> Occasionally, the assertion will trigger; is that something that I
>>>>>> should expect and handle?
>>>>>>
>>>>>> cheers,
>>>>>> James
>>>>>
>>>>
>>
>
