[GitHub] [geode] DonalEvans opened pull request #4749: Revert "GEODE-6536: Added retry in borrowConnection/single hop (#4719)"

["DonalEvans (GitHub)" <gi...@apache.org>]

This reverts commit 9da2cd49e2e04564b446eaad579b51e986bc2179.

Hangs were observed in internal tests and this commit was determined to be responsible. As of yet, no local reproduction of the issue is available, but work is ongoing to provide a test that can be used to debug the issue.

In addition to the hangs caused by this commit, it should be noted that no additional tests were checked in to validate the change, as required in the "For all changes" checklist that accompanies every PR. This alone could be seen as a reason to revert, as changes without associated tests to go with them should not really be merged in the first place.

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.


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

[GitHub] [geode] DonalEvans commented on issue #4749: Revert "GEODE-6536: Added retry in borrowConnection/single hop (#4719)"

["DonalEvans (GitHub)" <gi...@apache.org>]

I will let you know and provide a jira tickets soon as a reproduction of
the issue is available.

Regarding updating tests, it's true that changes were made to test files,
but only those changes required for the code to compile successfully
following a change to a method signature. When adding new code to introduce
some new behaviour (a retry timeout for borrowConnection() in this case),
it's very important to write new tests to verify both that the desired
behaviour is actually present, and to verify that no unintended side
effects have been introduced.

On Sat, Feb 29, 2020, 22:35 Mario Ivanac <no...@github.com> wrote:

> Could you inform us when test scenario which is hanging, is provided.
> Just info, that test cases where updated due to these changes, as required
> by "For all changes" checklist.
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/geode/pull/4749?email_source=notifications&email_token=AEEDGYIMGL7JWOSGS35RTQTRFH62XA5CNFSM4K6C6ZKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENMWTVI#issuecomment-593062357>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEEDGYKHVXVJE5XKVVBAIJTRFH62XANCNFSM4K6C6ZKA>
> .
>


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

[GitHub] [geode] DonalEvans closed pull request #4749: Revert "GEODE-6536: Added retry in borrowConnection/single hop (#4719)"

["DonalEvans (GitHub)" <gi...@apache.org>]

[ pull request closed by DonalEvans ]

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