You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Swapnil Bawaskar <sb...@apache.org> on 2015/08/25 19:37:36 UTC
Review Request 37765: Fix for GEODE-278
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/
-----------------------------------------------------------
Review request for geode and Darrel Schneider.
Repository: geode
Description
-------
While applying changes to the Region, pass in List for gathering pendingCallbacks
rather than a null on the remote members to get the same behavior as transaction host.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/37765/diff/
Testing
-------
Thanks,
Swapnil Bawaskar
Re: Review Request 37765: Fix for GEODE-278
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/#review96388
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java (line 158)
<https://reviews.apache.org/r/37765/#comment151729>
Adding a sys prop makes this seem like a feature instead of a bug.
Why not change to default behavior to have this bug fixed? Do you think the old way of doing it inline has value?
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java (line 4250)
<https://reviews.apache.org/r/37765/#comment151723>
do you want to change "bug 52530" to GEODE-278?
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java (line 4264)
<https://reviews.apache.org/r/37765/#comment151721>
clean up the "SWAP:" in multiple places in this test
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java (line 4304)
<https://reviews.apache.org/r/37765/#comment151722>
You should make sure this test always clears the system property. Even when the test fails earlier with an exception.
Could you do the sys prop clear at the end in its own finally block?
- Darrel Schneider
On Aug. 25, 2015, 10:37 a.m., Swapnil Bawaskar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37765/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 10:37 a.m.)
>
>
> Review request for geode and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> While applying changes to the Region, pass in List for gathering pendingCallbacks
> rather than a null on the remote members to get the same behavior as transaction host.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37765/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Swapnil Bawaskar
>
>
Re: Review Request 37765: Fix for GEODE-278
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/#review97163
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On Aug. 31, 2015, 2:26 p.m., Swapnil Bawaskar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37765/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2015, 2:26 p.m.)
>
>
> Review request for geode and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> While applying changes to the Region, pass in List for gathering pendingCallbacks
> rather than a null on the remote members to get the same behavior as transaction host.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37765/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Swapnil Bawaskar
>
>
Re: Review Request 37765: Fix for GEODE-278
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/#review97162
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java (line 740)
<https://reviews.apache.org/r/37765/#comment152942>
minor optimization: new ArrayList<>(this.farSideEntryOps.size())
to presize the pendingCallbacks array.
- Darrel Schneider
On Aug. 31, 2015, 2:26 p.m., Swapnil Bawaskar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37765/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2015, 2:26 p.m.)
>
>
> Review request for geode and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> While applying changes to the Region, pass in List for gathering pendingCallbacks
> rather than a null on the remote members to get the same behavior as transaction host.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37765/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Swapnil Bawaskar
>
>
Re: Review Request 37765: Fix for GEODE-278
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/#review99459
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On Sept. 17, 2015, 3:49 p.m., Swapnil Bawaskar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37765/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2015, 3:49 p.m.)
>
>
> Review request for geode and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> While applying changes to the Region, pass in List for gathering pendingCallbacks
> rather than a null on the remote members to get the same behavior as transaction host.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/ClientServerTransactionDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37765/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Swapnil Bawaskar
>
>
Re: Review Request 37765: Fix for GEODE-278
Posted by Swapnil Bawaskar <sb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/
-----------------------------------------------------------
(Updated Sept. 17, 2015, 10:49 p.m.)
Review request for geode and Darrel Schneider.
Changes
-------
GEODE-278 Enque events in one more spot.
With the previous commit, I had missed the code block for Adjunct messages. Enque tx events in that block too.
Repository: geode
Description
-------
While applying changes to the Region, pass in List for gathering pendingCallbacks
rather than a null on the remote members to get the same behavior as transaction host.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/ClientServerTransactionDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/37765/diff/
Testing
-------
Thanks,
Swapnil Bawaskar
Re: Review Request 37765: Fix for GEODE-278
Posted by Swapnil Bawaskar <sb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/
-----------------------------------------------------------
(Updated Aug. 31, 2015, 9:26 p.m.)
Review request for geode and Darrel Schneider.
Repository: geode
Description
-------
While applying changes to the Region, pass in List for gathering pendingCallbacks
rather than a null on the remote members to get the same behavior as transaction host.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/37765/diff/
Testing
-------
Thanks,
Swapnil Bawaskar
Re: Review Request 37765: Fix for GEODE-278
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/#review96415
-----------------------------------------------------------
Ship it!
Since you have changed the behavior instead of it being conditionally enabled make sure and run all the tests again.
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java (line 4321)
<https://reviews.apache.org/r/37765/#comment151753>
Since you got rid of the sys prop from the product shouldn't this go as well?
- Darrel Schneider
On Aug. 25, 2015, 1:01 p.m., Swapnil Bawaskar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37765/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 1:01 p.m.)
>
>
> Review request for geode and Darrel Schneider.
>
>
> Repository: geode
>
>
> Description
> -------
>
> While applying changes to the Region, pass in List for gathering pendingCallbacks
> rather than a null on the remote members to get the same behavior as transaction host.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37765/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Swapnil Bawaskar
>
>
Re: Review Request 37765: Fix for GEODE-278
Posted by Swapnil Bawaskar <sb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37765/
-----------------------------------------------------------
(Updated Aug. 25, 2015, 8:01 p.m.)
Review request for geode and Darrel Schneider.
Changes
-------
incorporated Darrel's feedback.
Repository: geode
Description
-------
While applying changes to the Region, pass in List for gathering pendingCallbacks
rather than a null on the remote members to get the same behavior as transaction host.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/TXCommitMessage.java f091570674f874539599b669af5d87290b329b18
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/RemoteTransactionDUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/37765/diff/
Testing
-------
Thanks,
Swapnil Bawaskar