accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 16081: ACCUMULO-1958 - Safer Range constructor
Date Fri, 06 Dec 2013 21:08:57 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message