> On Dec. 6, 2013, 8:35 p.m., kturner wrote:
> > src/core/src/main/java/org/apache/accumulo/core/data/Range.java, line 236
> > <https://reviews.apache.org/r/16081/diff/2/?file=395097#file395097line236>
> >
> >     I am thinking it might be a good idea to do the check beforeStartKey() 
> > check for this constructor and the readFields() method.  Thinking about the 
> > case of deserializing corrupted data.  readFields() is used in a similar 
> > way to a constructor.

I am not sure if this is worth doing.  Just something I thought of.  Given 
enough time and and enough machines this sanity check in deserialization would 
eventually catch an error.  But it comes at the cost of doing the check that 
was already done on the client side and assured by crcs at lower levels.  There 
is also the possibility that users will use these methods directly.  I know 
users write their own serializtion code for Key and Mutaton inorder to use map 
reduce and pipes.  


- kturner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16081/#review29907
-----------------------------------------------------------


On Dec. 6, 2013, 5:06 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16081/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2013, 5:06 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1958
>     https://issues.apache.org/jira/browse/ACCUMULO-1958
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Based on Sean's strategy, creates a new protected Range constructor without 
> the start/stop key check, and adds the check to the public six-argument 
> constructor. Opted not to deprecate the public constructor at this time, 
> since it is now safe.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/data/Range.java 
> 7ef0dc5710877cdd0dd3ead69e7db5d8c9ef68c1 
> 
> Diff: https://reviews.apache.org/r/16081/diff/
> 
> 
> Testing
> -------
> 
> Unit testing for Range still passes.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Reply via email to