accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Keith Turner (JIRA)" <>
Subject [jira] [Commented] (ACCUMULO-2300) Leaking internal state in core/client code
Date Fri, 31 Jan 2014 19:36:11 GMT


Keith Turner commented on ACCUMULO-2300:

A lot of this was probably intentional for performance reasons.  However, it can lead to bugs.
  For example the way mutation used to work was prone to bugs.  It kept a pointer to data
that users passed in.  If the user changed the byte array (usually a Text) after passing the
mutation to a batch writer it caused problems.   Now mutation serializes data as its added.
  The intent was to avoid a copy when data is added and a copy when its serialized to send
over the wire.  So if a choice is made to copy anywhere, its good consider the total number
of copies in the end to end data path.   

I really like the javadocs that [~bhavanki] added stating whats copied and not.  

> Leaking internal state in core/client code
> ------------------------------------------
>                 Key: ACCUMULO-2300
>                 URL:
>             Project: Accumulo
>          Issue Type: Improvement
>          Components: client
>    Affects Versions: 1.5.0
>            Reporter: Josh Elser
>            Priority: Minor
> In perusing over the findbugs reports, I noticed that there was a fair amount of classes
in the core module that were subject to leaking internal state (most commonly, returning the
actual array it holds internally instead of a copy or a wrapper).
> ArrayByteSequence, Column, ColumnUpdate, ComparableBytes, KeyValue, Mutation, Value,
ColumnVisibility are the classes that caught my eye.
> I would imagine that this is something we ultimately want to fix, but I'm not sure of
what the timeline should be. A decent short-term would be to just return copies of those {{byte[]}}
instead of the internal member. A longer-term solution might be to rework some of those classes
to use an object instead (e.g. ByteBuffer) that might have constructs to provide r/o views
instead of having to make a defensive copy.

This message was sent by Atlassian JIRA

View raw message