Skip to content

Conversation

gytis
Copy link

@gytis gytis commented Jul 4, 2017

Prior this pull request, database connection were created on DataSourceXAResourceRecoveryHelper.getXAResources() call and closed on DataSourceXAResourceRecoveryHelper.recover(TMENDRSCAN) call. While this workflow is still valid for most of the time, a change in one of Narayana recovery modules, allows commit/rollback calls to appear outside of this workflow. To make sure, that those calls will get through to the database, I've introduced a ConnectionManager, which makes sure that connection is created and closed whenever needed during recovery.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 4, 2017
@gytis gytis force-pushed the master-SB-107-narayana-jdbc-connection branch from b081c23 to ee80ade Compare July 5, 2017 10:31
@philwebb philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 5, 2017
@philwebb philwebb added this to the 2.0.0.M3 milestone Jul 5, 2017
@wilkinsona
Copy link
Member

I am not keen on merging this in its current form. The Narayana auto-configuration is getting more and more complex. I'd rather remove it in 2.0 if the extra complexity is unavoidable.

@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review status: on-hold We can't start working on this issue yet labels Jul 22, 2017
@wilkinsona wilkinsona removed this from the 2.0.0.M3 milestone Jul 22, 2017
@snicoll
Copy link
Member

snicoll commented Jul 22, 2017

Agreed. Can't we move the spring boot integration back to Narayana itself? I think I'd prefer if it would be released alongside the project rather than Spring Boot.

@gytis
Copy link
Author

gytis commented Jul 27, 2017

@snicoll I did ask @wilkinsona if this would be possible a couple of weeks back. I agree, that it would make things easier both for us and for you to keep things simple in here and keep the complexity in Narayana side. However, as @wilkinsona said this could be problematic in 1.5 brach, because he prefers to keep it on Narayana's 5.5.x branch. Narayana is currently on 5.6.x.

@snicoll
Copy link
Member

snicoll commented Jul 27, 2017

@gytis agreed. I was talking about Spring Boot 2 actually. This PR used to be assigned to Spring Boot 2 also...

@gytis
Copy link
Author

gytis commented Jul 27, 2017

Would it be doable for 1.5.x branch if I Narayana people would agree to take this logic for 5.5.x? This is just a speculation, I don't know if it's doable in their side.

@wilkinsona
Copy link
Member

@gytis It's hard to say without knowing the exact nature of the changes. 1.5.x is our long-term maintenance branch so we need to try to avoid anything that runs the risk of destabilising things.

@gytis
Copy link
Author

gytis commented Jul 31, 2017

@wilkinsona assuming that configuration with starter would stay here, we would remove 8 classes from this repo and make them available via Maven dependency. It shouldn't make any difference from the user point of view.

@gytis
Copy link
Author

gytis commented Aug 4, 2017

Closing this PR as it seems that guys in Narayana side were able to find a fix without modifying Spring Boot side. Thanks a lot for your time!
In a future I'll still look into slimming down this integration as discussed.

@gytis gytis closed this Aug 4, 2017
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review priority: normal status: on-hold We can't start working on this issue yet type: enhancement A general enhancement labels Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants