Re: Review Request 62506: GEODE-3679: Original client member Id in the transaction should be set in the PartitionMessage

2017-09-22 Thread Eric Shu 


> 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)
> > 
> >
> > 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)
> > 
> >
> > 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

2017-09-22 Thread Eric Shu 


> 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)
> > 
> >
> > 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

2017-09-22 Thread Eric Shu 

---
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

2017-09-22 Thread Nick Reich 

---
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)


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)


Avoid single character variable names



geode-core/src/test/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
Lines 4120 (patched)


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)


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
> 
>

Review Request 62506: GEODE-3679: Original client member Id in the transaction should be set in the PartitionMessage

2017-09-22 Thread Eric Shu 

---
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