[
https://issues.apache.org/jira/browse/GEODE-2891?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16072109#comment-16072109
]
ASF GitHub Bot commented on GEODE-2891:
---------------------------------------
Github user gregt5259 commented on the issue:
https://github.com/apache/geode-native/pull/105
I suggest to fix the issue by defining handshake pseudo message within the
range probably defined for such pseudo messages by original design i.e.:
typedef enum {
/* Server couldn't read message; handle it like a server side
exception that needs retries */
HANDSHAKE = -3
NOT_PUBLIC_API_WITH_TIMEOUT = -2,
WDYT?
Thanks,
Dr. Gregory Turovets
"…We're all mad here. I'm mad. You're mad."
"How do you know I'm mad?" said Alice.
"You must be," said the Cat, "or you wouldn't have come here."
Alice's Adventures in Wonderland by Lewis
Carroll<http://www.livelib.ru/author/157108>.
From: Jacob Barrett [mailto:notificati...@github.com]
Sent: Sunday, July 02, 2017 17:46
To: apache/geode-native <geode-nat...@noreply.github.com>
Cc: Gregory Turovets <gregory.turov...@amdocs.com>; Author
<aut...@noreply.github.com>
Subject: Re: [apache/geode-native] GEODE-2891 connect-timeout violation in
C++ Native Client (#105)
@pivotal-jbarrett requested changes on this pull request.
Please rebase your changes on develop rather than merge.
Do not include comments with ticket numbers.
Do not include comments with your name or initials.
Do not leave sources commented out, delete or delete not.
Please follow the C++ style of the community.
I am concerned with the approach of trying to define a pseudo message for
handshaking to get a different timeout unit. This may bite us in the future
when added new messages to the protocol.
There are a few tickets in flight, or soon to be in flight, that address
this problem.
https://issues.apache.org/jira/browse/GEODE-3136
https://issues.apache.org/jira/browse/GEODE-3137
I have begun some experiments with GEODE-3136 and should start committing
to it in a few days. All API exposed timeouts will be based on
std::chrono::duration so you can clearly see what unit of time your time is and
the code behind that API doesn't have to guess. GEODE-3137 will address on use
cases internally that aren't addressed when updating the public API. Any
configuration files that specify timeout will be updated to take a duration
string as well in the format of "1234s", "1234ms", etc.
________________________________
In
src/cppcache/src/TcrConnection.cpp<https://github.com/apache/geode-native/pull/105#discussion_r125184790>:
> @@ -318,7 +318,9 @@ bool TcrConnection::InitTcrConnection(
LOGFINE("Attempting handshake with endpoint %s for %s%s connection",
endpoint,
isClientNotification ? (isSecondary ? "secondary " : "primary ")
: "",
isClientNotification ? "subscription" : "client");
- ConnErrType error = sendData(data, msgLengh, connectTimeout, false);
+ // GT GEODE-2891
Do not include comments with your name or the ticket number.
________________________________
In
src/cppcache/src/TcrConnection.cpp<https://github.com/apache/geode-native/pull/105#discussion_r125184798>:
> @@ -318,7 +318,9 @@ bool TcrConnection::InitTcrConnection(
LOGFINE("Attempting handshake with endpoint %s for %s%s connection",
endpoint,
isClientNotification ? (isSecondary ? "secondary " : "primary ")
: "",
isClientNotification ? "subscription" : "client");
- ConnErrType error = sendData(data, msgLengh, connectTimeout, false);
+ // GT GEODE-2891
+ //ConnErrType error = sendData( data, msgLengh, connectTimeout, false );
Do not leave commented out sources, this is what revision control is for.
________________________________
In
src/cppcache/src/TcrConnection.cpp<https://github.com/apache/geode-native/pull/105#discussion_r125184917>:
> - // then app has set timeout in millis, change it to microSeconds
- sendTimeoutSec = sendTimeoutSec * 1000;
- isPublicApiTimeout = true;
- LOGDEBUG("sendData2 %d ", sendTimeoutSec);
- } else {
- sendTimeoutSec = sendTimeoutSec * 1000;
- }
+ notPublicApiWithTimeout ==
TcrMessage::EXECUTE_REGION_FUNCTION_SINGLE_HOP ||
+ // GT GEODE-2891
+ notPublicApiWithTimeout == TcrMessage::HANDSHAKE)
+ {
+ // then app has set timeout in millis, change it to microSeconds
+ sendTimeoutSec = sendTimeoutSec * 1000;
+ isPublicApiTimeout = true;
+ LOGDEBUG("sendData2 %d ", sendTimeoutSec);
+ }
Formatting does not conform to Google C++ Style
Guide<https://google.github.io/styleguide/cppguide.html>.
________________________________
In
src/cppcache/src/TcrMessage.hpp<https://github.com/apache/geode-native/pull/105#discussion_r125184959>:
> @@ -171,7 +173,8 @@ class CPPCACHE_EXPORT TcrMessage {
GET_DURABLE_CQS_DATA_ERROR = 106,
GET_ALL_WITH_CALLBACK = 107,
PUT_ALL_WITH_CALLBACK = 108,
- REMOVE_ALL = 109
+ REMOVE_ALL = 109,
+ HANDSHAKE = 110
These numbers correspond to protocol message numbers on the server. We can
just add one here and expect it not cause issues later.
________________________________
In
src/cppcache/src/TcrMessage.hpp<https://github.com/apache/geode-native/pull/105#discussion_r125184965>:
> @@ -44,6 +44,8 @@
#include <map>
#include <vector>
+//
Clean this up.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on
GitHub<https://github.com/apache/geode-native/pull/105#pullrequestreview-47551893>,
or mute the
thread<https://github.com/notifications/unsubscribe-auth/AbZcfm-Wvg_K8tzwTd2ACFihvkB1aQLaks5sJ60jgaJpZM4OLjA1>.
This message and the information contained herein is proprietary and
confidential and subject to the Amdocs policy statement,
you may review at https://www.amdocs.com/about/email-disclaimer
<https://www.amdocs.com/about/email-disclaimer>
> connect-timeout violation in C++ Native Client
> ----------------------------------------------
>
> Key: GEODE-2891
> URL: https://issues.apache.org/jira/browse/GEODE-2891
> Project: Geode
> Issue Type: Bug
> Components: native client
> Reporter: Gregory Vortman
> Attachments: GEODE-connect-timeout-violation.docx
>
>
> 1. C++ native client doesn’t honour read-timeout-milli-sec in a consistent
> way while connecting to a server
> 2. The lock on the connection pool has a very high granularity. Even if
> the client can’t connect to one server, all other threads which are working
> with totally different servers get affected by it
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
