[GitHub] [geode] pivotal-eshu opened pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["pivotal-eshu (GitHub)" <gi...@apache.org>]

 * Always send CommitProcessQueryMessage to other replicates to see if anyone received second 
   message as transaction originator for client transaction will be different from transaction host.
 * Do not wait for transaction originator to depart when processing CommitProcessQueryMessage if 
   transaction host is crashed. Originator may not be the same as the crashed transaction host and 
   never departs from the distributed system.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["pivotal-eshu (GitHub)" <gi...@apache.org>]

You are right that it is no longer in inner class. Cleaned up the code.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu closed pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["pivotal-eshu (GitHub)" <gi...@apache.org>]

[ pull request closed by pivotal-eshu ]

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

instead of "getDm()" use the parameter "distributionManager"

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

it would be better to just use the "distributionManager" parameter. At some point we may find that this class no longer needs to keep a reference to the dm in an instance field.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

rename "getDm" to "getDistributionManager"

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

What class is this method "doCommitProcessQuery" on? I think it is TXCommitMessage.
So no need for this funny business of "TXCommitMessage.this" which was needed before because is was in an anonymous inner class.
Now everywhere you have "message" you can just have "this".
I already did this refactoring in the pull request that removes thread groups.
I would suggest for your fix not to refactor this chunk of code into its own method. Just delete the block of code from it that you want. That way it will be more clear what you are fixing and you will not have conflicts with the thread refactoring code.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["pivotal-eshu (GitHub)" <gi...@apache.org>]

Will address this concern with additional dunit test.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

simplify to: return null != message && message.isProcessing();

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["pivotal-eshu (GitHub)" <gi...@apache.org>]

txTracker.commitProcessReceived method no longer need distribution manager anymore.

[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

I think the old code was better that passed along the "dm" parameter.


[ Full content available at: https://github.com/apache/geode/pull/2571 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org