accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 17426: ACCUMULO-1948 Tablet constructor no longer leaking this
Date Thu, 30 Jan 2014 20:09:03 GMT


> On Jan. 28, 2014, 3:49 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java,
line 358
> > <https://reviews.apache.org/r/17426/diff/3/?file=453017#file453017line358>
> >
> >     The same tablet can be unloaded and then reloaded on a tablet server.  It seems
like this change would make it possible for activity on a recently unloaded tablet to impact
another instance of itself that was just loaded.

I believe you're correct, and moreover I believe that it's possible even without this change.
The manageMemory() method snapshots the current tablet memory reports and asks the memory
manager what to do about them, and later grabs the current report for each tablet that should
be compacted. In between the snapshot and the grab, the tablet could have been unloaded and
reloaded, in which case any report found would be for the new instance. (When the old instance
closes, its report is removed.)

So, I think to overcome this I need to restore the tablet reference into the memory report
(TabletState), but also, the manageMemory() method needs to grab from its snapshotted reports,
not whatever the current ones are. When it gets the tablet reference from the report, it will
still check if the tablet has been closed, which covers the unload/reload scenario. How does
that sound?


- Bill


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


On Jan. 28, 2014, 3:10 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17426/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 3:10 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 40dfcf0fa1715fbb0ff69e78bdf8954146a36b20

>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 5c1f6ce5f95523ef16005902c99a8f974e9e2baf

>   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. Real cluster testing TBD.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


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