You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2016/04/12 01:05:16 UTC

Review Request 46060: fix the off-heap leak in netWrite

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

Review request for geode, Scott Jewell, Ken Howe, and Sai Boorlagadda.


Bugs: GEODE-1199
    https://issues.apache.org/jira/browse/GEODE-1199


Repository: geode


Description
-------

doNetWrite now calls release when it creates a new EntryEventImpl.
Added a unit test for doNetWrite.


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessor.java 8224fc2826cb7650a812cef6e04f399000211593 
  geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/46060/diff/


Testing
-------

precheckin


Thanks,

Darrel Schneider

Re: Review Request 46060: fix the off-heap leak in netWrite

Posted by Ken Howe <kh...@pivotal.io>.

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


Ship it!




Ship It!

- Ken Howe


On April 11, 2016, 11:05 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46060/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 11:05 p.m.)
> 
> 
> Review request for geode, Scott Jewell, Ken Howe, and Sai Boorlagadda.
> 
> 
> Bugs: GEODE-1199
>     https://issues.apache.org/jira/browse/GEODE-1199
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> doNetWrite now calls release when it creates a new EntryEventImpl.
> Added a unit test for doNetWrite.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessor.java 8224fc2826cb7650a812cef6e04f399000211593 
>   geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46060/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 46060: fix the off-heap leak in netWrite

Posted by Scott Jewell <sj...@pivotal.io>.

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


Ship it!




Ship It!

- Scott Jewell


On April 11, 2016, 11:05 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46060/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 11:05 p.m.)
> 
> 
> Review request for geode, Scott Jewell, Ken Howe, and Sai Boorlagadda.
> 
> 
> Bugs: GEODE-1199
>     https://issues.apache.org/jira/browse/GEODE-1199
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> doNetWrite now calls release when it creates a new EntryEventImpl.
> Added a unit test for doNetWrite.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessor.java 8224fc2826cb7650a812cef6e04f399000211593 
>   geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46060/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 46060: fix the off-heap leak in netWrite

Posted by Darrel Schneider <ds...@pivotal.io>.


> On April 12, 2016, 3:07 p.m., Ken Howe wrote:
> > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java, lines 59-62
> > <https://reviews.apache.org/r/46060/diff/1/?file=1340084#file1340084line59>
> >
> >     I think it be good to verify that EntryEvetnImpl.release() is called in addition to verifying that retain and release are called the correct number of times? Can you spy EntryEventImpl to accomplish this.

No, the EntryEventImpl that the test knows about is not released by this code. A copy of it is made internally and it is that one that is released. The identity of that one is not visible to this test. All I care about in this test is that we don't leak the StoredObject.


- Darrel


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


On April 11, 2016, 4:05 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46060/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 4:05 p.m.)
> 
> 
> Review request for geode, Scott Jewell, Ken Howe, and Sai Boorlagadda.
> 
> 
> Bugs: GEODE-1199
>     https://issues.apache.org/jira/browse/GEODE-1199
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> doNetWrite now calls release when it creates a new EntryEventImpl.
> Added a unit test for doNetWrite.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessor.java 8224fc2826cb7650a812cef6e04f399000211593 
>   geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46060/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 46060: fix the off-heap leak in netWrite

Posted by Ken Howe <kh...@pivotal.io>.

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




geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java (lines 59 - 62)
<https://reviews.apache.org/r/46060/#comment191983>

    I think it be good to verify that EntryEvetnImpl.release() is called in addition to verifying that retain and release are called the correct number of times? Can you spy EntryEventImpl to accomplish this.


- Ken Howe


On April 11, 2016, 11:05 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46060/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 11:05 p.m.)
> 
> 
> Review request for geode, Scott Jewell, Ken Howe, and Sai Boorlagadda.
> 
> 
> Bugs: GEODE-1199
>     https://issues.apache.org/jira/browse/GEODE-1199
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> doNetWrite now calls release when it creates a new EntryEventImpl.
> Added a unit test for doNetWrite.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessor.java 8224fc2826cb7650a812cef6e04f399000211593 
>   geode-core/src/test/java/com/gemstone/gemfire/internal/cache/SearchLoadAndWriteProcessorTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46060/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>