You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by James Peach <ja...@me.com> on 2012/01/01 07:07:36 UTC

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

Re: inconsistent read IOBuffer results

Posted by James Peach <ja...@me.com>.
On 02/01/2012, at 6:04 PM, Brian Geffon 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();

avail is a int64_t*. This is just avoiding the NULL dereference if avail is not provided.

> 
> 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 :

I'm not sure I follow this. I understood that the bytes are available to be read until they are consumed by TSIOBufferReaderConsume(). So on the call to TSIOBufferBlockReadStart(), the returned available byte count must be less than or equal to the result of TSIOBufferBlockReadAvail(). It would only every be less than if the data is split over multiple buffer blocks.

> 
>  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?

somewhat :)

> 
> On Mon, Jan 2, 2012 at 5:45 PM, James Peach <ja...@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 [jamespeach@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
>>> 
>> 


Re: inconsistent read IOBuffer results

Posted by James Peach <ja...@me.com>.
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 [jamespeach@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" <br...@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 <ja...@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 [jamespeach@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
>>>>> 
>>>> 
>> 
> 


RE: inconsistent read IOBuffer results

Posted by Brian Geffon <bg...@linkedin.com>.
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.


Brian

________________________________________
From: James Peach [jamespeach@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" <br...@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 <ja...@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 [jamespeach@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
>>>>
>>>
>


Re: inconsistent read IOBuffer results

Posted by James Peach <ja...@me.com>.
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" <br...@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 <ja...@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 [jamespeach@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
>>>> 
>>> 
> 


Re: inconsistent read IOBuffer results

Posted by Brian Geffon <bg...@linkedin.com>.
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().
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);
   }

Brian



On 1/2/12 6:04 PM, "Brian Geffon" <br...@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 <ja...@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 [jamespeach@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
>>>
>>


Re: inconsistent read IOBuffer results

Posted by Brian Geffon <br...@gmail.com>.
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 <ja...@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 [jamespeach@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
>>
>

Re: inconsistent read IOBuffer results

Posted by James Peach <ja...@me.com>.
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 [jamespeach@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
> 


Re: inconsistent read IOBuffer results

Posted by James Peach <ja...@me.com>.
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.

thanks,
James

> 
> Brian
> ________________________________________
> From: James Peach [jamespeach@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


RE: inconsistent read IOBuffer results

Posted by Brian Geffon <bg...@linkedin.com>.
I think you might want TSIOBufferBlockReadAvail and not TSIOBufferReaderAvail.

Brian
________________________________________
From: James Peach [jamespeach@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