[GitHub] [tinkerpop] dalaro opened a new pull request #1113: TINKERPOP-2216 add status attribute for warnings
dalaro opened a new pull request #1113: TINKERPOP-2216 add status attribute for
warnings
URL: https://github.com/apache/tinkerpop/pull/1113
JIRA link: https://issues.apache.org/jira/browse/TINKERPOP-2216
There are a lot of alternative ways to approach this. I'm just opening this
PR, with this specific approach, because it illustrates the suggested feature a
bit more clearly than the prose in the JIRA issue description.
This adds a new {{Tokens}} string literal with the value `"warnings"`. The
console's `DriverRemoteAcceptor` checks for this status attribute every time it
receives a remote resultset, printing the value above the results (to `err`
when available). If the value is a list, each element gets passed through
`String#valueOf` and printed on a separate line. Otherwise, it's just passed
through `String#valueOf` and printed once.
Testing this in isolation seems a bit tricky. I've been testing this
coupled with a remote graph running in Gremlin-Server. If anyone has
suggestions about reusable test harness bits underneath
`gremlin-console/src/test` that I should reuse for this, I'd be interested.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] divijvaidya commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
divijvaidya commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-491974093 No, there is no reason to merge this before the 3.4.2 release. We can merge it after that. Meanwhile, I will continue adding changes to this branch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-491969136 > I agree with targeting this change for v3.4.3. :+1: > If any providers have provided a custom channelizer by extending AbstractChannelizer, they will have to modify their channelizer implementation. Is there a way we can inform providers of such implementations? You should write an entry in the upgrade documentation: https://github.com/apache/tinkerpop/blob/master/docs/src/upgrade/release-3.4.x.asciidoc > Is there anything else left (from an implementation perspective) that you would like me to change? I will address the documentation/examples about the working of the client in a separate PR. I will be doing some manual tests this week. Since we can't merge this until after we go through our release process, it should be fine to keep pushing changes to this PR (i.e. the documentation updates). I don't really like to break up doc fixes from code changes, if possible. Is there a reason you would like to see it merge ahead of those changes? since we're waiting for 3.4.3 now there is a bit of a delay to merge anyway right? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] divijvaidya commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
divijvaidya commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-491966179 1. I agree with targeting this change for v3.4.3. 2. If any providers have provided a custom channelizer by extending AbstractChannelizer, they will have to modify their channelizer implementation. Is there a way we can inform providers of such implementations? 3. Is there anything else left (from an implementation perspective) that you would like me to change? I will address the documentation/examples about the working of the client in a separate PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] newkek commented on issue #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode
newkek commented on issue #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode URL: https://github.com/apache/tinkerpop/pull/1110#issuecomment-491949258 VOTE +1 (non-binding) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] divijvaidya commented on issue #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode
divijvaidya commented on issue #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode URL: https://github.com/apache/tinkerpop/pull/1110#issuecomment-491939208 VOTE +1 : Code is OK, Semantics of this change is OK This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[tinkerpop] branch master updated (7c73810 -> dfca86c)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from 7c73810 Merge branch 'tp33' add dfca86c TINKERPOP-2190 Added docs on Gremlin injection for scripts CTR No new revisions were added by this update. Summary of changes: docs/src/reference/gremlin-applications.asciidoc | 49 +--- 1 file changed, 44 insertions(+), 5 deletions(-)
[tinkerpop] branch tp33 updated (dfe9a93 -> 783eb07)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch tp33 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from dfe9a93 Merge remote-tracking branch 'origin/TINKERPOP-2206' into tp33 add 783eb07 TINKERPOP-2198 Update store() documentation wrt EarlyLimitStrategy CTR No new revisions were added by this update. Summary of changes: docs/src/reference/the-traversal.asciidoc | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-)
[tinkerpop] branch master updated: TINKERPOP-2198 Update store() documentation wrt EarlyLimitStrategy CTR
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master by this push:
new 783eb07 TINKERPOP-2198 Update store() documentation wrt
EarlyLimitStrategy CTR
new 7c73810 Merge branch 'tp33'
783eb07 is described below
commit 783eb0758fe5ae3fc9804438cb641bab9b4efc84
Author: Stephen Mallette
AuthorDate: Mon May 13 11:14:01 2019 -0400
TINKERPOP-2198 Update store() documentation wrt EarlyLimitStrategy CTR
---
docs/src/reference/the-traversal.asciidoc | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/docs/src/reference/the-traversal.asciidoc
b/docs/src/reference/the-traversal.asciidoc
index 88a20b5..d6b281a 100644
--- a/docs/src/reference/the-traversal.asciidoc
+++ b/docs/src/reference/the-traversal.asciidoc
@@ -2572,11 +2572,13 @@ stores objects in its side-effect collection as they
pass through.
[gremlin-groovy,modern]
-g.V().aggregate('x').limit(1).cap('x')
+g.V().aggregate('x').limit(1).cap('x')
g.V().store('x').limit(1).cap('x')
+g.withoutStrategies(EarlyLimitStrategy).V().store('x').limit(1).cap('x')
-It is interesting to note that there are two results in the `store()`
side-effect even though the interval
+It is important to note that `EarlyLimitStrategy` introduced in 3.3.5 alters
the behavior of `store()`. Without that
+strategy (which is installed by default), there are two results in the
`store()` side-effect even though the interval
selection is for 1 object. Realize that when the second object is on its way
to the `range()` filter (i.e. `[0..1]`),
it passes through `store()` and thus, stored before filtered.
[tinkerpop] branch TINKERPOP-2089 updated (d8353f4 -> 52bfdfe)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch TINKERPOP-2089 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from d8353f4 TINKERPOP-2089 Added docs for javascript DSL add 52bfdfe TINKERPOP-2089 Minor fix to update docs No new revisions were added by this update. Summary of changes: docs/src/upgrade/release-3.3.x.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
[GitHub] [tinkerpop] newkek commented on a change in pull request #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode
newkek commented on a change in pull request #1110: TINKERPOP-2211 Add API
which allows per-request option for bytecode
URL: https://github.com/apache/tinkerpop/pull/1110#discussion_r283374742
##
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteConnection.java
##
@@ -40,6 +40,16 @@
public static final String GREMLIN_REMOTE = "gremlin.remote.";
public static final String GREMLIN_REMOTE_CONNECTION_CLASS =
GREMLIN_REMOTE + "remoteConnectionClass";
+/**
+ * Key for configuring a per-request timeout option for a {@code
RemoteConnection} .
+ */
+public static final String PER_REQUEST_TIMEOUT = GREMLIN_REMOTE +
"timeout";
Review comment:
> I guess if you were a user and you knew that certain config options were
the same then you could just treat the config as a string key and then it would
work without any additional dependency...
Yes, the point of that comment was that you'd want to use the official
Tinkerpop name for the options to parse them. Falling back to using equivalent
string values does not protect from if these values change in a Tinkerpop
update. But I understand your other point for locating that stuff explicitly in
`gremlin-driver`/`gremlin-server`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[tinkerpop] branch TINKERPOP-2089 updated (bc004a5 -> d8353f4)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch TINKERPOP-2089 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from bc004a5 TINKERPOP-2089 Added tests for anoynmous DSL steps add d8353f4 TINKERPOP-2089 Added docs for javascript DSL No new revisions were added by this update. Summary of changes: CHANGELOG.asciidoc| 1 + docs/src/reference/the-traversal.asciidoc | 62 +++ docs/src/upgrade/release-3.3.x.asciidoc | 11 +- 3 files changed, 73 insertions(+), 1 deletion(-)
[tinkerpop] branch TINKERPOP-2089 updated (22e3560 -> bc004a5)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch TINKERPOP-2089 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from 22e3560 TINKERPOP-2089 Minor fixes add bc004a5 TINKERPOP-2089 Added tests for anoynmous DSL steps No new revisions were added by this update. Summary of changes: .../test/integration/traversal-test.js | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-)
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1110: TINKERPOP-2211 Add API which allows per-request option for bytecode
spmallette commented on a change in pull request #1110: TINKERPOP-2211 Add API
which allows per-request option for bytecode
URL: https://github.com/apache/tinkerpop/pull/1110#discussion_r283325413
##
File path:
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Tokens.java
##
@@ -34,6 +34,7 @@ private Tokens() {}
public static final String OPS_KEYS = "keys";
public static final String OPS_CLOSE = "close";
+public static final String ARGS_OVERRIDE_REQUEST_ID = "overrideRequestId";
Review comment:
agreed - done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[tinkerpop] branch TINKERPOP-2211 updated (9d7bceb -> 128662b)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch TINKERPOP-2211 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from 9d7bceb TINKERPOP-2211 Changes around bytecode submission of RequestOptions add 128662b TINKERPOP-2211 Minor renaming of requestId setting for RequestOptions No new revisions were added by this update. Summary of changes: .../src/main/java/org/apache/tinkerpop/gremlin/driver/Tokens.java | 2 +- .../tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java | 6 +++--- .../tinkerpop/gremlin/driver/remote/DriverRemoteConnectionTest.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
[GitHub] [tinkerpop] jorgebay commented on issue #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
jorgebay commented on issue #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript URL: https://github.com/apache/tinkerpop/pull/1112#issuecomment-491804045 I've pushed a commit to the dev branch, feel free to override. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[tinkerpop] branch TINKERPOP-2089 updated: TINKERPOP-2089 Minor fixes
This is an automated email from the ASF dual-hosted git repository.
jorgebg pushed a commit to branch TINKERPOP-2089
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/TINKERPOP-2089 by this push:
new 22e3560 TINKERPOP-2089 Minor fixes
22e3560 is described below
commit 22e35606688f41278390cf6683db2740474b4250
Author: Jorge Bay Gondra
AuthorDate: Mon May 13 14:30:16 2019 +0200
TINKERPOP-2089 Minor fixes
---
gremlin-javascript/glv/GraphTraversalSource.template| 11 ++-
.../gremlin-javascript/lib/process/anonymous-traversal.js | 5 +
.../gremlin-javascript/lib/process/graph-traversal.js | 11 ++-
.../javascript/gremlin-javascript/lib/structure/graph.js| 7 ---
.../gremlin-javascript/test/integration/traversal-test.js | 13 ++---
5 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/gremlin-javascript/glv/GraphTraversalSource.template
b/gremlin-javascript/glv/GraphTraversalSource.template
index d96279a..9f94828 100644
--- a/gremlin-javascript/glv/GraphTraversalSource.template
+++ b/gremlin-javascript/glv/GraphTraversalSource.template
@@ -22,23 +22,25 @@
*/
'use strict';
-const Traversal = require('./traversal').Traversal;
+const { Traversal } = require('./traversal');
const remote = require('../driver/remote-connection');
const utils = require('../utils');
const Bytecode = require('./bytecode');
-const TraversalStrategies =
require('./traversal-strategy').TraversalStrategies;
+const { TraversalStrategies } = require('./traversal-strategy');
/**
* Represents the primary DSL of the Gremlin traversal machine.
*/
class GraphTraversalSource {
+
/**
+ * Creates a new instance of {@link GraphTraversalSource}.
* @param {Graph} graph
* @param {TraversalStrategies} traversalStrategies
* @param {Bytecode} [bytecode]
- * @param {Class} graphTraversalSourceClass
- * @param {Class} graphTraversalClass
+ * @param {Function} [graphTraversalSourceClass] Optional {@link
GraphTraversalSource} constructor.
+ * @param {Function} [graphTraversalClass] Optional {@link GraphTraversal}
constructor.
*/
constructor(graph, traversalStrategies, bytecode, graphTraversalSourceClass,
graphTraversalClass) {
this.graph = graph;
@@ -55,7 +57,6 @@ class GraphTraversalSource {
withRemote(remoteConnection) {
const traversalStrategy = new
TraversalStrategies(this.traversalStrategies);
traversalStrategy.addStrategy(new remote.RemoteStrategy(remoteConnection));
-var object = Object.create(this.constructor.prototype);
return new this.graphTraversalSourceClass(this.graph, traversalStrategy,
new Bytecode(this.bytecode), this.graphTraversalSourceClass,
this.graphTraversalClass);
}
diff --git
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
index 3fbe7eb..ff30a4b 100644
---
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
+++
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
@@ -31,6 +31,10 @@ const Graph = require('../structure/graph').Graph;
*/
class AnonymousTraversalSource {
+ /**
+ * Creates a new instance of {@link AnonymousTraversalSource}.
+ * @param {Function} [traversalSourceClass] Optional {@code
GraphTraversalSource} constructor.
+ */
constructor(traversalSourceClass) {
this.traversalSourceClass = traversalSourceClass;
}
@@ -38,6 +42,7 @@ class AnonymousTraversalSource {
/**
* Constructs an {@code AnonymousTraversalSource} which will then be
configured to spawn a
* {@link GraphTraversalSource}.
+ * @param {Function} [traversalSourceClass] Optional {@code
GraphTraversalSource} constructor.
* @returns {AnonymousTraversalSource}.
*/
static traversal(traversalSourceClass) {
diff --git
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js
index 89285e3..34e06db 100644
---
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js
+++
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js
@@ -22,23 +22,25 @@
*/
'use strict';
-const Traversal = require('./traversal').Traversal;
+const { Traversal } = require('./traversal');
const remote = require('../driver/remote-connection');
const utils = require('../utils');
const Bytecode = require('./bytecode');
-const TraversalStrategies =
require('./traversal-strategy').TraversalStrategies;
+const { TraversalStrategies } = require('./traversal-strategy');
/**
* Represents the primary DSL of the Gremlin traversal machine.
*/
class GraphTraversalSource {
+
/**
+ * Creates a
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL
support to gremlin-javascript
URL: https://github.com/apache/tinkerpop/pull/1112#discussion_r283250472
##
File path:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
##
@@ -31,13 +31,17 @@ const Graph = require('../structure/graph').Graph;
*/
class AnonymousTraversalSource {
+ constructor(traversalSourceClass) {
+this.traversalSourceClass = traversalSourceClass;
+ }
+
/**
* Constructs an {@code AnonymousTraversalSource} which will then be
configured to spawn a
* {@link GraphTraversalSource}.
* @returns {AnonymousTraversalSource}.
*/
- static traversal() {
-return new AnonymousTraversalSource();
+ static traversal(traversalSourceClass) {
Review comment:
We should document in jsdoc: `@param {Function} [traversalSourceClass]
Optional GraphTraversalSource constructor.`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
spmallette commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491791395 i see - now that you say this, i feel like you've told me this before - sorry, if that's the case. i'll try again to get Microsoft attention on this. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel
spmallette commented on issue #1105: TINKERPOP-2205 Change connection management to single request per channel URL: https://github.com/apache/tinkerpop/pull/1105#issuecomment-491790730 > which, from a users perspective maintains the same behaviour as the old configuration, i.e. executing 16x32 queries in parallel. Thanks for this clarification. I see it now - I agree. > I am afraid the "switch" is going to be a boolean controlling if/else branches across the code. hmm - well, unless there aren't many such if/else situations such a change might just make the whole thing messy, but i can't imagine that we'd get lucky that way. Maybe you don't need to try to do all that. How do you feel about keeping this targeted at 3.4.x but not merging for the release of 3.4.2 next week? If we did that, we would have ~45 days of testing, review, etc for 3.4.3 (release would be middle/end of july given our current two month release cycles which I believe would continue). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491788326 @spmallette > does Gremlin.NET need a keep-alive option? Gremlin.Net relies on the underlying WebSocket keepalive implementation which sends by default every 30 seconds a ping. According to [this blog post](https://blogs.msdn.microsoft.com/whereismysolution/2018/03/12/does-system-net-websockets-includes-a-keepalive-mechanism-which-automatically-takes-care-of-pingpong-control-frames/), it also responds to ping messages coming from the server. That is why I'm wondering why Azure apparently still terminates the connections. It seems to ignore those keepalive pings for its check whether a connection is still alive. But a PCAP file that actually shows the network traffic would give us clarity that a) pings are actually sent and answered and that b) Azure is actively terminating the connection which leads to the problem. @danielcweber > I tested it with a network card reset, killing all connections, just like we assume CosmosDB to. The appropriate exception is then thrown. Yes, that's of course expected. But that doesn't tell us whether CosmosDB terminated the connections, although that's basically the only explanation I can think of. But to decide on a course of action we would also need to know why CosmosDB terminates connections. Does it really ignore keepalive pings or are they not sent / answered for some reason? > I think Gremlin.net should not wait for the next query to recover, but try right away. It doesn't try it right away because the assumption was that connections were closed because the server is currently not available at all. In that case, it makes more sense in my opinion to wait for the next query and try then whether the server is available again (e.g., because it was restarted). In your case where the connections were closed because they were inactive, it could make sense to make an attempt at creating new connections right away instead of throwing an exception. I would however prefer if we could avoid this situation from occurring in the first place instead of trying to work around it. If we can't avoid it for some reason, then we could make an attempt at creating new connections when all connections were closed before we throw an exception. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491781771 >So, the pool should recover automatically, even after throwing a ConnectionPoolBusyException. Ok, there's a misunderstanding. I cannot confirm that the pool doesn't ever recover. It does probably. What happens is, we definitely hit 5. in your enumeration. I tested it with a network card reset, killing all connections, just like we assume CosmosDB to. The appropriate exception is then thrown. I think Gremlin.net should not wait for the next query to recover, but try right away. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491781771 >So, the pool should recover automatically, even after throwing a ConnectionPoolBusyException. Ok, there's maybe a misunderstanding. I cannot confirm that the pool doesn't ever recover. It does probably. What happens is, we definitely hit 5. in your enumeration. I tested it with a network card reset, killing all connections, just like we assume CosmosDB to. The appropriate exception is then thrown. I think Gremlin.net should not wait for the next query to recover, but try right away. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
spmallette commented on issue #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript URL: https://github.com/apache/tinkerpop/pull/1112#issuecomment-491781120 @jorgebay I'll wait for you to push the fixes you intended to add. I just realized though that I didn't handle testing anonymous traversals. I still need to go back in and do that. > but after seeing how clean a custom DSL would look like (SocialTraversal and SocialTraversalSource), I think it makes sense to match the functionality in the other GLVs great! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
spmallette commented on a change in pull request #1112: TINKERPOP-2089 Added
DSL support to gremlin-javascript
URL: https://github.com/apache/tinkerpop/pull/1112#discussion_r283296753
##
File path: gremlin-javascript/glv/GraphTraversalSource.template
##
@@ -51,7 +55,8 @@ class GraphTraversalSource {
withRemote(remoteConnection) {
const traversalStrategy = new
TraversalStrategies(this.traversalStrategies);
traversalStrategy.addStrategy(new remote.RemoteStrategy(remoteConnection));
-return new GraphTraversalSource(this.graph, traversalStrategy, new
Bytecode(this.bytecode));
+var object = Object.create(this.constructor.prototype);
Review comment:
yeah - that should be deleted.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
spmallette commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491779855 > Our current assumption is still that the connections are closed by Azure when they are idle for a while. @FlorianHockmann does Gremlin.NET need a keep-alive option? I assume we don't see this problem in true Gremlin Server implementations because of the server-side keep-alive that is sending pings to the client? does Gremlin.NET even respond to those? if not, i'm not sure i understand why we don't see it in Gremlin Server too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491779093 > Quick question before I dig into the sources: It sounds as if Gremlin.net does not re-open or re-create closed connections. True? It does create new connections, but it throws an exception first for failed requests. The creation of new connections is actually what was fixed by this PR, so you'll have to build yourself from `master`. The method `EnsurePoolIsPopulatedAsync` is executed for every incoming request and it fills the pool with new connections if it's at its configured size. Replacing closed connections works like this: 1. Query comes in 1. `ConnectionPool` searches for an available connection (`GetConnectionFromPool`) 1. A closed connection is encountered (in `TryGetAvailableConnection`). 1. The closed connection is removed from the pool 1. If no connection is available (e.g., because all connections were closed), a `ConnectionPoolBusyException` is thrown. 1. Another query comes in 1. `EnsurePoolIsPopulatedAsync` notices that the pool is empty and populates it again So, the pool should recover automatically, even after throwing a `ConnectionPoolBusyException`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491761136 >Is it possible to create a PCAP for debugging with Azure Functions Probably out of luck for PCAP and Azure Functions, but I can do it locally (there's nothing specific to Azure Functions). >So, is the connection idle for some time before the exception gets thrown Definitely - the exception just happens after a day or more, and there's nothing really going on at the moment on that service, so this is most probably the case. Quick question before I dig into the sources: It sounds as if Gremlin.net does not re-open or re-create closed connections. True? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491761136 >Is it possible to create a PCAP for debugging with Azure Functions Probably out of luck for PCAP and Azure Functions, but I can do it locally (there's nothing specific to Azure Functions). >So, is the connection idle for some time before the exception gets thrown Definitely - the exception just happens after a day or more, and there's nothing really going on at the moment on that service, so this is most probably the case. Quick question before I dig into the sources: It sounds as if Gremlin.net does not re-open or re-create closed connections. True? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL
support to gremlin-javascript
URL: https://github.com/apache/tinkerpop/pull/1112#discussion_r283248976
##
File path: gremlin-javascript/glv/GraphTraversalSource.template
##
@@ -51,7 +55,8 @@ class GraphTraversalSource {
withRemote(remoteConnection) {
const traversalStrategy = new
TraversalStrategies(this.traversalStrategies);
traversalStrategy.addStrategy(new remote.RemoteStrategy(remoteConnection));
-return new GraphTraversalSource(this.graph, traversalStrategy, new
Bytecode(this.bytecode));
+var object = Object.create(this.constructor.prototype);
Review comment:
Looks like a leftover.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL support to gremlin-javascript
jorgebay commented on a change in pull request #1112: TINKERPOP-2089 Added DSL
support to gremlin-javascript
URL: https://github.com/apache/tinkerpop/pull/1112#discussion_r283250472
##
File path:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/anonymous-traversal.js
##
@@ -31,13 +31,17 @@ const Graph = require('../structure/graph').Graph;
*/
class AnonymousTraversalSource {
+ constructor(traversalSourceClass) {
+this.traversalSourceClass = traversalSourceClass;
+ }
+
/**
* Constructs an {@code AnonymousTraversalSource} which will then be
configured to spawn a
* {@link GraphTraversalSource}.
* @returns {AnonymousTraversalSource}.
*/
- static traversal() {
-return new AnonymousTraversalSource();
+ static traversal(traversalSourceClass) {
Review comment:
We should document in jsdoc: `@param {GraphTraversalSource}
[traversalSourceClass] Optional GraphTraversalSource constructor.`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
FlorianHockmann commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491730080 > All 4 connections have reached their MaxInProcessPerConnection limit of 32. Consider increasing either the PoolSize or the MaxInProcessPerConnection limit. This exception is currently thrown if all connections reached their limit (for which case the text makes sense) **or** if all connections in the pool are already in _Closed_ state and where therefore removed from the pool while trying to find an available connection. I just created [TINKERPOP-2215](https://issues.apache.org/jira/browse/TINKERPOP-2215) to throw a different exception in the latter case. As a starting point to further dig into this, a PCAP file could be helpful to check what happens right before the exception gets thrown. Is it possible to create a PCAP for debugging with Azure Functions? Our current assumption is still that the connections are closed by Azure when they are idle for a while. It would be good to know whether that's actually the case. So, is the connection idle for some time before the exception gets thrown? In that case we could evaluate how to proceed. I can think of a few different options, but I don't know enough about the problem right now to really evaluate which approach makes most sense. I would especially like to have a confirmation that Azure closes connections even if a ping message is sent regularly which we do by default in Gremlin.Net exactly to avoid problems like this one. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491709887 Unfortunately, I can't confirm the issue to be solved. We still run into pool draining on an Azure Function. We use CosmosDB. The message is: All 4 connections have reached their MaxInProcessPerConnection limit of 32. Consider increasing either the PoolSize or the MaxInProcessPerConnection limit. @FlorianHockmann This has previously been linked to CosmosDB, which may or may not be the cause. Let me know how I can help, I will free up the resources to get this examined further. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber edited a comment on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491709887 Unfortunately, I can't confirm the issue to be solved. We still run into pool draining on an Azure Function. We use CosmosDB. The message is: All 4 connections have reached their MaxInProcessPerConnection limit of 32. Consider increasing either the PoolSize or the MaxInProcessPerConnection limit. @FlorianHockmann This has previously been linked to CosmosDB, which may or may not be the cause. Let me know how I can help, I will free up the ressources to get this examined further. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [tinkerpop] danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated
danielcweber commented on issue #1096: TINKERPOP-2090: Fix Sign for Checking if Connection Pool Needs to be Repopulated URL: https://github.com/apache/tinkerpop/pull/1096#issuecomment-491709887 Unfortunately, I can't confirm the issue to be solved. We still run into pool draining on an Azure Function. We use CosmosDB. The message is: All 4 connections have reached their MaxInProcessPerConnection limit of 32. Consider increasing either the PoolSize or the MaxInProcessPerConnection limit. @FlorianHockmann This has previously been linked to CosmosDB, which may or may not be the cause. Let me know how I can help, I will free up the ressources to help getting this examined. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
