Correct release notes

[dominikzalewski]

Description

Reliability of connecting to data sources was improved in all JDBC based connectors by adding retries. The Oracle connector already supported retries, so the release notes should apply to all JDBC based connectors except Oracle.

Release notes

( X ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

[hashhar]

while this is "technically true", we both know the change in current shape is a no-op for Postgresql because it's driver doesn't throw the exception on which we retry.
Same goes for other connectors.

It might actually be less misleading to have the existing release notes because we know for a fact that retries work for Oracle, we can't say the same about any of the others on the list though.

I obviously don't have strong opinions here so I'll defer to other JDBC experts @ebyhr @wendigo .

[dominikzalewski]

@hashhar, if I understand you correctly, you are actually opting for having no release notes for this change, as for Oracle we kept it backward compatible, so there is actually no change for this database.

[nineinchnick]

the change in current shape is a no-op for Postgresql because it's driver doesn't throw the exception on which we retry.
Same goes for other connectors.

So the whole change is not effective for any connector?

[dominikzalewski]

So the whole change is not effective for any connector?

For Oracle it is effective, the same way as it was in the previous code.
I've checked explicitly for PostgreSQL and it is NOT effective, because they never throw SQLTransientException.
For others, we've never checked whether the drivers throw SQLTransientException or not, so WE DON'T KNOW whether it affects the other connectors or not.

[nineinchnick]

If we haven't checked it, then we can't claim to have improved reliability.

[dominikzalewski]

Let's just remove it entirely from release notes. Files commited.

[mosabua]

@martint and myself tried to decipher the PR and suggested release note to understand what is actually happening. Our understanding was that it adds a new framework for all JDBC connectors that is inactive. But it does change the behavior for Oracle adjusting to the different exception. Since this is all too much detail for a release notes entry we adjusted to the short notice. If there really is no user-facing effect from the PR and it is just an internal refactor.. then I am okay with deleting this. But you will need to confirm this @dominikzalewski

And my follow up questions would then be ... are you sending follow up PRs that take advantage of the new framework .. and if not .. why did we do the refactor? From a users perspective...

[hashhar]

are you sending follow up PRs that take advantage of the new framework .. and if not .. why did we do the refactor?

The refactor was to unify on a single way to do retries across all JDBC connectors. Right now we had multiple places where those could be done leading to lot of duplication, special casing, copy-pasting when and if we wanted to add to new connectors.

This lays the groundwork for being able to send follow-up PRs for specific connectors as needed.

As and when we notice complains/issues in the wild/with users about opening connections we'll add retries for those. Without knowing when a retry is needed and how the failure modes look like we cannot blindly add retries.

And yes the change seems to be a no-op from user's perspective (I didn't realise this correctly when I asked Dominik to provide the release notes) for now.

[mosabua]

Given the discussion in this PR the removal makes sense.

[hashhar]

Sorry about being the source of the confusion. 🙂

Thanks @dominikzalewski for noticing and sending the correction.