You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by bschuchardt <gi...@git.apache.org> on 2017/03/03 22:06:44 UTC
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
GitHub user bschuchardt opened a pull request:
https://github.com/apache/geode/pull/412
GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOneWithSSLAndTheOther\u2026
This was a product issue. When the locator using plain-text sockets is
contacted by a TcpClient using SSL the locator often just closes the socket.
On some platforms this causes a SSLHandshakeException but on others it
just causes a "SocketException: connection reset". Writing some text to
the socket forces the TcpClient to get a SSLException (which is the superclass
of SSLHandshakeException).
The test class is still marked as Flaky due to GEODE-2542.
I deleted one of the tests in LocatorDUnitTest as it wasn't doing any useful validation and really served no purpose.
I also increased the joinTimeout in this test. The original 1-second timeout was intended to make the tests run faster but I think it's probably the source of some of the flaky-ness in this set of tests. Some of them were also overriding the joinTimeout established by the DUnit framework, so that was actually a bad thing to be doing. The tests all run in a few seconds with the default joinTimeout setting anyway.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/geode feature/GEODE-1793
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/412.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #412
----
commit 866dc5ca1583c5fab49ec96c48d261c0367427f3
Author: Bruce Schuchardt <bs...@pivotal.io>
Date: 2017-03-03T21:47:42Z
GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOneWithSSLAndTheOtherNonSSL
This was a product issue. When the locator using plain-text sockets is
contacted by a TcpClient using SSL the locator often just closes the socket.
On some platforms this causes a SSLHandshakeException but on others it
just causes a "SocketException: connection reset". Writing some text to
the socket forces the TcpClient to get a SSLException (which is the superclass
of SSLHandshakeException).
The test class is still marked as Flaky due to GEODE-2542.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt closed the pull request at:
https://github.com/apache/geode/pull/412
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/412#discussion_r104258974
--- Diff: geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java ---
@@ -77,9 +77,9 @@
* <p>
* This should be incremented if the gossip message structures change
* <p>
- * 1000 - gemfire 5.5 - using java serialization 1001 - 5.7 - using DataSerializable and
--- End diff --
Did spotless break this? \U0001f62e
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/412#discussion_r104258182
--- Diff: geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java ---
@@ -360,6 +360,13 @@ private void processRequest(final Socket sock) {
versionOrdinal = (short) GOSSIP_TO_GEMFIRE_VERSION_MAP.get(gossipVersion);
} else {
// Close the socket. We can not accept requests from a newer version
+ try {
+ sock.getOutputStream().write("unknown protocol version".getBytes());
--- End diff --
Is there any risk of this being interpreted by anything other than garbage on the other side?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOneWithSSL...
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/412
There are "spotless" problems I'm cleaning up, and I'm removing the commented out code from the test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on a diff in the pull request:
https://github.com/apache/geode/pull/412#discussion_r104260160
--- Diff: geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java ---
@@ -360,6 +360,13 @@ private void processRequest(final Socket sock) {
versionOrdinal = (short) GOSSIP_TO_GEMFIRE_VERSION_MAP.get(gossipVersion);
} else {
// Close the socket. We can not accept requests from a newer version
+ try {
+ sock.getOutputStream().write("unknown protocol version".getBytes());
--- End diff --
This situation should never happen. When we fetch the protocol version of a Locator we use v5.7. Then we use the locator's protocol version in future communications. We will never send a higher version number than the locator knows how to handle.
For SSL, this text is nothing like the response to a HelloClient packet. On machines I've tested on it results in either a SSLHandshakeException or a SSLException hinting that the other end might be using plain text sockets.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOne...
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on a diff in the pull request:
https://github.com/apache/geode/pull/412#discussion_r104259153
--- Diff: geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java ---
@@ -77,9 +77,9 @@
* <p>
* This should be incremented if the gossip message structures change
* <p>
- * 1000 - gemfire 5.5 - using java serialization 1001 - 5.7 - using DataSerializable and
--- End diff --
I didn't check but that's my assumption. It likes the comment with the HTML breaks in place.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #412: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOneWithSSL...
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/412
remote: geode git commit: GEODE_1793 spotless fixes and removal of dead code
remote: geode git commit: GEODE-1793 LocatorDUnitTest.testStartTwoLocatorsOneWithSSLAndTheOtherNonSSL
To https://git-wip-us.apache.org/repos/asf/geode.git
c09a856..4112204 develop -> develop
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---