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/09/22 17:01:31 UTC
Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/
-----------------------------------------------------------
Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
Bugs: GEODE-3679
https://issues.apache.org/jira/browse/GEODE-3679
Repository: geode
Description
-------
Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
Diff: https://reviews.apache.org/r/62506/diff/1/
Testing
-------
precheckin.
Thanks,
Eric Shu
Re: Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
Posted by Eric Shu <es...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/
-----------------------------------------------------------
(Updated Sept. 22, 2017, 7:12 p.m.)
Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
Changes
-------
Update review comments.
Bugs: GEODE-3679
https://issues.apache.org/jira/browse/GEODE-3679
Repository: geode
Description
-------
Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
Diff: https://reviews.apache.org/r/62506/diff/2/
Changes: https://reviews.apache.org/r/62506/diff/1-2/
Testing
-------
precheckin.
Thanks,
Eric Shu
Re: Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
Posted by Eric Shu <es...@pivotal.io>.
> On Sept. 22, 2017, 6:27 p.m., Nick Reich wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
> > Lines 4109 (patched)
> > <https://reviews.apache.org/r/62506/diff/1/?file=1832888#file1832888line4109>
> >
> > This is actually "doTransactionOnServer", as there is nothing about the method which is tied to server1.
client is connect to server1, tx should be on server1.
- Eric
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/#review186003
-----------------------------------------------------------
On Sept. 22, 2017, 7:12 p.m., Eric Shu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62506/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2017, 7:12 p.m.)
>
>
> Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
>
>
> Bugs: GEODE-3679
> https://issues.apache.org/jira/browse/GEODE-3679
>
>
> Repository: geode
>
>
> Description
> -------
>
> Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
> geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
>
>
> Diff: https://reviews.apache.org/r/62506/diff/2/
>
>
> Testing
> -------
>
> precheckin.
>
>
> Thanks,
>
> Eric Shu
>
>
Re: Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
Posted by Nick Reich <nr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/#review186003
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4109 (patched)
<https://reviews.apache.org/r/62506/#comment262355>
This is actually "doTransactionOnServer", as there is nothing about the method which is tied to server1.
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4114 (patched)
<https://reviews.apache.org/r/62506/#comment262351>
Avoid single character variable names
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4120 (patched)
<https://reviews.apache.org/r/62506/#comment262354>
Can this have a generic type like above method? Would remove casting to Integer below.
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4125 (patched)
<https://reviews.apache.org/r/62506/#comment262353>
Do not need to hold onto the set:
for (Object key : region.keySet())
- Nick Reich
On Sept. 22, 2017, 5:01 p.m., Eric Shu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62506/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2017, 5:01 p.m.)
>
>
> Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
>
>
> Bugs: GEODE-3679
> https://issues.apache.org/jira/browse/GEODE-3679
>
>
> Repository: geode
>
>
> Description
> -------
>
> Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
> geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
>
>
> Diff: https://reviews.apache.org/r/62506/diff/1/
>
>
> Testing
> -------
>
> precheckin.
>
>
> Thanks,
>
> Eric Shu
>
>
Re: Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
Posted by Eric Shu <es...@pivotal.io>.
> On Sept. 22, 2017, 6:55 p.m., anilkumar gingade wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
> > Lines 4111 (patched)
> > <https://reviews.apache.org/r/62506/diff/1/?file=1832888#file1832888line4111>
> >
> > Do we need to check for expected trasaction id here...
The server1 transaction id does not have issue and does not change. The issue is that server2 receives the keySet op from server1 and will start a TXStateProxy there. There is nothing in the test could get hold of it, however, the test would fail without this fix.
> On Sept. 22, 2017, 6:55 p.m., anilkumar gingade wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
> > Lines 4115 (patched)
> > <https://reviews.apache.org/r/62506/diff/1/?file=1832888#file1832888line4115>
> >
> > Do we need sleep...
Needed to make sure the race could occur -- keySet op executed during this transactions commit.
- Eric
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/#review186005
-----------------------------------------------------------
On Sept. 22, 2017, 7:12 p.m., Eric Shu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62506/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2017, 7:12 p.m.)
>
>
> Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
>
>
> Bugs: GEODE-3679
> https://issues.apache.org/jira/browse/GEODE-3679
>
>
> Repository: geode
>
>
> Description
> -------
>
> Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
> geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
>
>
> Diff: https://reviews.apache.org/r/62506/diff/2/
>
>
> Testing
> -------
>
> precheckin.
>
>
> Thanks,
>
> Eric Shu
>
>
Re: Review Request 62506: GEODE-3679: Original client member Id in the
transaction should be set in the PartitionMessage
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62506/#review186005
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4111 (patched)
<https://reviews.apache.org/r/62506/#comment262357>
Do we need to check for expected trasaction id here...
geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4115 (patched)
<https://reviews.apache.org/r/62506/#comment262356>
Do we need sleep...
- anilkumar gingade
On Sept. 22, 2017, 5:01 p.m., Eric Shu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62506/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2017, 5:01 p.m.)
>
>
> Review request for geode, anilkumar gingade, Darrel Schneider, Kirk Lund, Lynn Gallinat, and Nick Reich.
>
>
> Bugs: GEODE-3679
> https://issues.apache.org/jira/browse/GEODE-3679
>
>
> Repository: geode
>
>
> Description
> -------
>
> Sets the originating member id from client transaction in partition message when a server needs to perform operaton and send to other peers.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionMessage.java 8c27107
> geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java 96b89b9
>
>
> Diff: https://reviews.apache.org/r/62506/diff/1/
>
>
> Testing
> -------
>
> precheckin.
>
>
> Thanks,
>
> Eric Shu
>
>