[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17722059#comment-17722059
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
FlorianHockmann merged PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17721954#comment-17721954
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
kenhuuu commented on PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058#issuecomment-1544900944
VOTE +1
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17721820#comment-17721820
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
vkagamlyk commented on PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058#issuecomment-1544163982
LGTM
VOTE +1
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17721818#comment-17721818
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
FlorianHockmann commented on PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058#issuecomment-1544154709
I made the change you suggested, @vkagamlyk. But I also had to make another
change as my fix wasn't complete. It did solve the problem that the
`Connection` class had an ever increasing list of cancellation registrations,
but those registrations weren't disposed and were therefore still stored in the
underlying `CancellationTokenSource`.
We've found that by trying out the fix in the service where we initially
found the memory leak. The earlier fix here already improved the situation a
lot but a memory dump revealed that the `CancellationTokenSource` had a linked
list of all cancellation registrations.
The version that is now in this PR doesn't show any increasing memory usage
any more (at least right now after executing the application for ~30min which
was already enough earlier to see the problem, but I will update this comment
tomorrow to confirm this).
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720998#comment-17720998
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
vkagamlyk commented on code in PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058#discussion_r1188864810
##
gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs:
##
@@ -175,6 +175,7 @@ private void
HandleReceivedMessage(ResponseMessage> receivedMsg)
{
responseHandler?.Finalize(status.Attributes);
_callbackByRequestId.TryRemove(receivedMsg.RequestId.Value,
out _);
+
_cancellationByRequestId.TryRemove(receivedMsg.RequestId.Value, out _);
Review Comment:
maybe it's better to move clearing of `_cancellationByRequestId` 2 lines
higher to avoid leak when `responseHandler?.Finalize` failed?
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720827#comment-17720827
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
codecov-commenter commented on PR #2058:
URL: https://github.com/apache/tinkerpop/pull/2058#issuecomment-1539604114
##
[Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2058?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)
Report
> Merging
[#2058](https://app.codecov.io/gh/apache/tinkerpop/pull/2058?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)
(27f7dbd) into
[3.5-dev](https://app.codecov.io/gh/apache/tinkerpop/commit/2ee47f203ffa76b9c0c466bd8ba291010afdd831?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)
(2ee47f2) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@Coverage Diff @@
## 3.5-dev#2058 +/- ##
==
Coverage 69.38% 69.39%
+ Complexity 8975 8974-1
==
Files866 866
Lines 4125141251
Branches5442 5442
==
+ Hits 2862128625+4
+ Misses 1071910716-3
+ Partials1911 1910-1
```
[see 6 files with indirect coverage
changes](https://app.codecov.io/gh/apache/tinkerpop/pull/2058/indirect-changes?src=pr=tree-more_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD
build times. [Learn
more](https://about.codecov.io/iterative-testing/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (TINKERPOP-2944) Memory leak in Gremlin.Net driver if CancellationToken is used
[
https://issues.apache.org/jira/browse/TINKERPOP-2944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17720809#comment-17720809
]
ASF GitHub Bot commented on TINKERPOP-2944:
---
FlorianHockmann opened a new pull request, #2058:
URL: https://github.com/apache/tinkerpop/pull/2058
https://issues.apache.org/jira/browse/TINKERPOP-2944
This should fix the memory leak described in TINKERPOP-2944 caused by not
cleaning up the cancellation token registrations which contain a reference to
the `RequestMessage`.
VOTE +1
> Memory leak in Gremlin.Net driver if CancellationToken is used
> --
>
> Key: TINKERPOP-2944
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2944
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
>Affects Versions: 3.6.2, 3.5.5, 3.6.3, 3.5.6
>Reporter: Florian Hockmann
>Assignee: Florian Hockmann
>Priority: Major
>
> We have noticed in my team at G DATA that a .NET service has been using an
> increasing amount of memory since we began to propagate the
> {{CancellationToken}} into the Gremlin.Net traversal executions which was
> introduced in TINKERPOP-2794.
> We could track this down with memory profiling to [this place in the
> driver|https://github.com/apache/tinkerpop/blob/5c8af70c7fac3ee98df36d250b05320e67ff1b96/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L94]
> where we register a callback to execute when cancellation is requested.
> These registered callbacks are only cleaned up when the whole {{Connection}}
> object gets disposed. Since the callback contains a reference to the full
> {{RequestMessage}} of the traversal, it can contain a lot of data. This is
> something that I've completely overlooked when I added support for
> cancellation.
> The driver should clean up those callbacks when they are not needed any more
> because the request has been processed completely (either successfully or
> with an error).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
