You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2017/03/23 01:45:45 UTC
Review Request 57864: GEODE-2472 changes are made to ensure byte
array is completely flushed to oplog channel.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57864/
-----------------------------------------------------------
Review request for geode, Darrel Schneider and Eric Shu.
Repository: geode
Description
-------
GEODE-2472:
The Oplog and OverflowOplog flush(OplogFile olf, ByteBuffer b1, ByteBuffer b2) method doesn't check the results of the channel.write() call.
It could so happen partial byte array is written to the channel. The check is added to make sure the bytes are completely written to the file channel.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2
geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java c7f8e0d
geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java d24e66d
Diff: https://reviews.apache.org/r/57864/diff/1/
Testing
-------
Run new unit tests added.
precheckin in progress.
Thanks,
anilkumar gingade
Re: Review Request 57864: GEODE-2472 changes are made to ensure byte
array is completely flushed to oplog channel.
Posted by anilkumar gingade <ag...@pivotal.io>.
> On March 23, 2017, 6:55 p.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java
> > Line 756 (original), 766 (patched)
> > <https://reviews.apache.org/r/57864/diff/1/?file=1672428#file1672428line767>
> >
> > I'm ok with you changing this method to set bbArray[0] and then null it out
> > BUT I think if you do this then you also need to change "consumeWriteBuf" and "createCrf" to not change bbArray.
The new changes posted has changes to "consumeWriteBuf" and "createCrf" (not change bbarray).
- anilkumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57864/#review169911
-----------------------------------------------------------
On March 23, 2017, 1:45 a.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57864/
> -----------------------------------------------------------
>
> (Updated March 23, 2017, 1:45 a.m.)
>
>
> Review request for geode, Darrel Schneider and Eric Shu.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2472:
> The Oplog and OverflowOplog flush(OplogFile olf, ByteBuffer b1, ByteBuffer b2) method doesn't check the results of the channel.write() call.
>
> It could so happen partial byte array is written to the channel. The check is added to make sure the bytes are completely written to the file channel.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2
> geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java c7f8e0d
> geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java d24e66d
>
>
> Diff: https://reviews.apache.org/r/57864/diff/1/
>
>
> Testing
> -------
>
> Run new unit tests added.
> precheckin in progress.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 57864: GEODE-2472 changes are made to ensure byte
array is completely flushed to oplog channel.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57864/#review169911
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java
Line 756 (original), 766 (patched)
<https://reviews.apache.org/r/57864/#comment242619>
I'm ok with you changing this method to set bbArray[0] and then null it out
BUT I think if you do this then you also need to change "consumeWriteBuf" and "createCrf" to not change bbArray.
- Darrel Schneider
On March 22, 2017, 6:45 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57864/
> -----------------------------------------------------------
>
> (Updated March 22, 2017, 6:45 p.m.)
>
>
> Review request for geode, Darrel Schneider and Eric Shu.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2472:
> The Oplog and OverflowOplog flush(OplogFile olf, ByteBuffer b1, ByteBuffer b2) method doesn't check the results of the channel.write() call.
>
> It could so happen partial byte array is written to the channel. The check is added to make sure the bytes are completely written to the file channel.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2
> geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java c7f8e0d
> geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java d24e66d
>
>
> Diff: https://reviews.apache.org/r/57864/diff/1/
>
>
> Testing
> -------
>
> Run new unit tests added.
> precheckin in progress.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 57864: GEODE-2472 changes are made to ensure byte
array is completely flushed to oplog channel.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57864/#review169934
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On March 23, 2017, 2:59 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57864/
> -----------------------------------------------------------
>
> (Updated March 23, 2017, 2:59 p.m.)
>
>
> Review request for geode, Darrel Schneider and Eric Shu.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2472:
> The Oplog and OverflowOplog flush(OplogFile olf, ByteBuffer b1, ByteBuffer b2) method doesn't check the results of the channel.write() call.
>
> It could so happen partial byte array is written to the channel. The check is added to make sure the bytes are completely written to the file channel.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2
> geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java c7f8e0d
> geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java d24e66d
>
>
> Diff: https://reviews.apache.org/r/57864/diff/2/
>
>
> Testing
> -------
>
> Run new unit tests added.
> precheckin in progress.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 57864: GEODE-2472 changes are made to ensure byte
array is completely flushed to oplog channel.
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57864/
-----------------------------------------------------------
(Updated March 23, 2017, 9:59 p.m.)
Review request for geode, Darrel Schneider and Eric Shu.
Changes
-------
Added review comment changes.
Repository: geode
Description
-------
GEODE-2472:
The Oplog and OverflowOplog flush(OplogFile olf, ByteBuffer b1, ByteBuffer b2) method doesn't check the results of the channel.write() call.
It could so happen partial byte array is written to the channel. The check is added to make sure the bytes are completely written to the file channel.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2
geode-core/src/main/java/org/apache/geode/internal/cache/OverflowOplog.java c7f8e0d
geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java d24e66d
Diff: https://reviews.apache.org/r/57864/diff/2/
Changes: https://reviews.apache.org/r/57864/diff/1-2/
Testing
-------
Run new unit tests added.
precheckin in progress.
Thanks,
anilkumar gingade