Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============7564875345546369538=="
MIME-Version: 1.0
Subject: Re: Review Request 17426: ACCUMULO-1948 Tablet constructor no longer
 leaking
 this
From: keith@deenlo.com
To: "Bill Havanki" <bhavanki@clouderagovt.com>,
 "accumulo" <dev@accumulo.apache.org>, keith@deenlo.com
Date: Mon, 03 Feb 2014 23:56:53 -0000
Message-ID: <20140203235653.3575.36576@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: noreply@reviews.apache.org
References: <20140203234956.3575.58745@reviews.apache.org>
In-Reply-To: <20140203234956.3575.58745@reviews.apache.org>
Reply-To: keith@deenlo.com

--===============7564875345546369538==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> On Feb. 3, 2014, 11:49 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java, line 356
> > <https://reviews.apache.org/r/17426/diff/4/?file=457847#file457847line356>
> >
> >     It seems like this warning could have occurred spuriously before your change to copy the map.  So your change may be a bug fix, unrelated to fixing the "this" issue.
> >     
> >     This case of tabletReport being null seems like something that should never happen now.  So maybe remove the null check and should it ever happen let the NPE occur.

Actually my comment about tabletReport not being null, assumes a well behaved memory manager (which is pluggable).   So maybe the warning should stay.   


- kturner


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


On Jan. 31, 2014, 7:28 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17426/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 7:28 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1948
>     https://issues.apache.org/jira/browse/ACCUMULO-1948
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Change in the construction of the cross-referencing between Tablet and TabletResourceManager to avoid the Tablet constructor leaking this.
> 
> 
> Diffs
> -----
> 
>   server/tserver/pom.xml b627de092faa2b5b4dbc5f8b9e4fb7af696ca436 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 5ea2401acc720c70d58ee04c2dd82dc79fcf2b99 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 9d08c81f9b07a8c5f8a281769fd2914f5c8fc82b 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java e95843775c85ecf90a2b4c7afce91094381da450 
>   server/tserver/src/test/java/org/apache/accumulo/tserver/TabletResourceManagerTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/17426/diff/
> 
> 
> Testing
> -------
> 
> Added unit test. Tested on pseudo-cluster with shell activity, randomwalk MultiTable test.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


--===============7564875345546369538==--