You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Eric Shu <es...@pivotal.io> on 2017/03/15 20:14:31 UTC

Review Request 57660: GEODE-2536: Do not change keyId for persistent region DiskId

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

Review request for geode, anilkumar gingade and Darrel Schneider.


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


Repository: geode


Description
-------

markForWriting and unmarkForWriting should not used for persistent region.
needsToBeWritten always return false now for persistent region, as the diskEntry either is being written to disk (sync) or sheduled to be written to disk (async)


Diffs
-----

  geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java bde6a32 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java e802ac9 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java cce8100 
  geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java 195c6a2 


Diff: https://reviews.apache.org/r/57660/diff/1/


Testing
-------

precheckin.


Thanks,

Eric Shu


Re: Review Request 57660: GEODE-2536: Do not change keyId for persistent region DiskId

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57660/#review169063
-----------------------------------------------------------


Ship it!




Ship It!

- Darrel Schneider


On March 15, 2017, 2:10 p.m., Eric Shu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57660/
> -----------------------------------------------------------
> 
> (Updated March 15, 2017, 2:10 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and Darrel Schneider.
> 
> 
> Bugs: GEODE-2536
>     https://issues.apache.org/jira/browse/GEODE-2536
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> markForWriting and unmarkForWriting should not used for persistent region.
> needsToBeWritten always return false now for persistent region, as the diskEntry either is being written to disk (sync) or sheduled to be written to disk (async)
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java bde6a32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java e802ac9 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java cce8100 
>   geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java 195c6a2 
> 
> 
> Diff: https://reviews.apache.org/r/57660/diff/2/
> 
> 
> Testing
> -------
> 
> precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>


Re: Review Request 57660: GEODE-2536: Do not change keyId for persistent region DiskId

Posted by Eric Shu <es...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57660/
-----------------------------------------------------------

(Updated March 15, 2017, 9:10 p.m.)


Review request for geode, anilkumar gingade and Darrel Schneider.


Changes
-------

fix review comment


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


Repository: geode


Description
-------

markForWriting and unmarkForWriting should not used for persistent region.
needsToBeWritten always return false now for persistent region, as the diskEntry either is being written to disk (sync) or sheduled to be written to disk (async)


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java bde6a32 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java e802ac9 
  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java cce8100 
  geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java 195c6a2 


Diff: https://reviews.apache.org/r/57660/diff/2/

Changes: https://reviews.apache.org/r/57660/diff/1-2/


Testing
-------

precheckin.


Thanks,

Eric Shu


Re: Review Request 57660: GEODE-2536: Do not change keyId for persistent region DiskId

Posted by Eric Shu <es...@pivotal.io>.

> On March 15, 2017, 8:53 p.m., anilkumar gingade wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
> > Line 1691 (original), 1691 (patched)
> > <https://reviews.apache.org/r/57660/diff/1/?file=1665716#file1665716line1691>
> >
> >     We should remove this comment as it doesn't make sense for Geode...

This will be addressed when fixing GEODE-2535.


> On March 15, 2017, 8:53 p.m., anilkumar gingade wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
> > Line 706 (original), 706 (patched)
> > <https://reviews.apache.org/r/57660/diff/1/?file=1665718#file1665718line706>
> >
> >     How about using, "isKeyIdNegative"...

This will not be applied once the GEODE-2535 is fixed. KeyId will be final for persistent region. No need to change for this checkin.


- Eric


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


On March 15, 2017, 8:14 p.m., Eric Shu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57660/
> -----------------------------------------------------------
> 
> (Updated March 15, 2017, 8:14 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and Darrel Schneider.
> 
> 
> Bugs: GEODE-2536
>     https://issues.apache.org/jira/browse/GEODE-2536
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> markForWriting and unmarkForWriting should not used for persistent region.
> needsToBeWritten always return false now for persistent region, as the diskEntry either is being written to disk (sync) or sheduled to be written to disk (async)
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java bde6a32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java e802ac9 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java cce8100 
>   geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java 195c6a2 
> 
> 
> Diff: https://reviews.apache.org/r/57660/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>


Re: Review Request 57660: GEODE-2536: Do not change keyId for persistent region DiskId

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57660/#review169056
-----------------------------------------------------------




geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
Line 1691 (original), 1691 (patched)
<https://reviews.apache.org/r/57660/#comment241368>

    We should remove this comment as it doesn't make sense for Geode...



geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java
Line 561 (original), 561 (patched)
<https://reviews.apache.org/r/57660/#comment241370>

    is it appropriate to say, not needed for persistent region...Or remove the comment as exception indicates that.



geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
Line 706 (original), 706 (patched)
<https://reviews.apache.org/r/57660/#comment241371>

    How about using, "isKeyIdNegative"...


- anilkumar gingade


On March 15, 2017, 8:14 p.m., Eric Shu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57660/
> -----------------------------------------------------------
> 
> (Updated March 15, 2017, 8:14 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and Darrel Schneider.
> 
> 
> Bugs: GEODE-2536
>     https://issues.apache.org/jira/browse/GEODE-2536
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> markForWriting and unmarkForWriting should not used for persistent region.
> needsToBeWritten always return false now for persistent region, as the diskEntry either is being written to disk (sync) or sheduled to be written to disk (async)
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java bde6a32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java e802ac9 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java cce8100 
>   geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java 195c6a2 
> 
> 
> Diff: https://reviews.apache.org/r/57660/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>