Fix releasing of resources in case connection initialization failed

[nmarton-da]

Previously InitHookDataSourceProxy was not closing the connection if initialization failed,
which causing worker-locks to stuck on PostgreSQL side, preventing subsequent HA initialization.
This PR fixes by closing explicitly the connection, if initialization failed.

changelog_begin
changelog_end

Pull Request Checklist

• Read and understand the contribution guidelines
• Include appropriate tests
• Set a descriptive title and thorough description
• Add a reference to the issue this PR will solve, if appropriate
• Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
• Normal production system change, include purpose of change in description
• If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

[rautenrieth-da]

The change looks good, left a comment about rewriting the code in a more Scala style.

[rautenrieth-da]

You could also use Using:

Using(connectionBody) { connection => 
  initHook(connection)
}.fold(t => throw t, identity) 

• Connection is AutoCloseable, so should be closed by Using
• Using also handles exceptions in connectionBody (unlike the current code)

Not sure what happens if just the closing of the connection fails though, we probably do not care about that error.

[kamil-da]

It's a lot more easier to read in the form @rautenrieth-da proposed.

[nmarton-da]

won't Using actually close the connection anyway?

[oliverse-da]

To the extent that this helps us clear the canton toxiproxy testing, this fix would be a candidate for porting to 1.18.x as part of a 1.18.1 release. This has come up at canton standup today - cc @daravep . I have labeled this with the need-backport label to keep this on our radar.

[nmarton-da]

To the extent that this helps us clear the canton toxiproxy testing, this fix would be a candidate for porting to 1.18.x as part of a 1.18.1 release. This has come up at canton standup today - cc @daravep . I have labeled this with the need-backport label to keep this on our radar.

Thanks for the important note @oliverse-da
Would it be too much to ask to verify this fix in your toxiproxy testing environment first? (given this is the second try to fix that issue 😊 ) cc @phoebe-da

[oliverse-da]

Thanks for the important note @oliverse-da Would it be too much to ask to verify this fix in your toxiproxy testing environment first? (given this is the second try to fix that issue 😊 ) cc @phoebe-da

When tomorrow's daml 2.0.0 snapshot comes out, I will produce a "dry-run" upgrade to daml 2.0.0 (dry run because canton will not upgrade to daml 2.0.0 for at least another month or more). I suppose then we could run the toxiproxy tests against the resulting dry-run/upgrade branch.