Mailing-List: contact notifications-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jira@apache.org
Date: Wed, 13 Feb 2013 16:50:12 +0000 (UTC)
From: "Keith Turner (JIRA)" <jira@apache.org>
To: notifications@accumulo.apache.org
Message-ID: <JIRA.12611345.1349962202918.284971.1360774212529@arcas>
In-Reply-To: <JIRA.12611345.1349962202918@arcas>
References: <JIRA.12611345.1349962202918@arcas>
Subject: [jira] [Commented] (ACCUMULO-803) Add Reverse Logical Time as a
 Time Type
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/ACCUMULO-803?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13577704#comment-13577704 ] 

Keith Turner commented on ACCUMULO-803:
---------------------------------------

This is a nice patch, I had not looked at it in a while.  Code, test, and documentation!  I did a thorough review and found some issues with the patch :

 * When a tablet is flushed it does not save reverse time properly.  This is because of a check tablet does to make sure time increases.  Search for persistedTime in tablet.  The greater than check against persisted time should probably be abstracted out to tablet time.
 * The initial time on a tables initial tablet is not set correctly, its set to "D0".  Should be "D"+Long.MAX.  MetadataTable.addTablet() should call TabletTime to ask for initial metadata value for time type.
 * ReverseLogicalTime constructor should not subtract from max long. This constructor is called every time tablet loads.  Setting initial tables tablet will make things work properly.
 * Merges of tablets are taking maximum time instead of minimum time.  TabletTime.maxMetadataTime() should take reverse action for reverse time.
 * Need to test bulk import w/ reverse logical time

Sorry I dropped the ball on this.  We discussed all of theses other options and I got sidetracked and never did an in depth review.  

Drew, I think your original idea is good.  It is a simple addition to the existing model.  The other possible changes discussed require completely rethinking the existing model.  Adding it would not prevent some of the more complex changes discussed being implemented in the future.  

As far as 1.5, this patch was in way before 1.5 feature freeze date.  I would kinda like to see it go in 1.5, but this is partially because I feel guilty for dropping the ball on this.  On the other hand, I do want to see work on 1.5 really start to settle and focus solely on bug fixes.

I will attach a little program I used to poke at this patch.
                
> Add Reverse Logical Time as a Time Type
> ---------------------------------------
>
>                 Key: ACCUMULO-803
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-803
>             Project: Accumulo
>          Issue Type: Improvement
>          Components: tserver
>    Affects Versions: 1.5.0
>            Reporter: Drew Farris
>            Assignee: Drew Farris
>            Priority: Minor
>         Attachments: ACCUMULO-803.patch, ACCUMULO-803.patch
>
>
> In a context where we are doing aggregation/combination of multiple values for a given key it may be useful to iterate over the values associated with that key in the order in which the mutations were applied (FIFO), instead of the FILO order that seems to occur when using {{TimeType.LOGICAL}}. 
> I encountered when implemeting a checkAndPut operation that would ensure that the previous value was expected before putting a new value. In this case, if the previous value was not as expected, the mutation would be ignored. 
> Perhaps it is useful in a general case?

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira