You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/08/10 09:17:42 UTC
[GitHub] [pulsar-dotpulsar] PetterIsberg opened a new pull request #85: Add a watchdog to the connections
PetterIsberg opened a new pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85
To solve the issue were a consumer gets stuck trying to receive a message from the Pulsar server due to network drops, this PR adds a watchdog.
The watchdog is disabled by default, but can be enabled by setting a watchdog timeout in the PulsarClientBuilder.
The watchdog is "kicked" every time the connection receives a Pulsar message. If no message is received within the timeout timespan, a timer is cancelling a CancellationToken, aborting the receive attempt.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] PetterIsberg commented on a change in pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
PetterIsberg commented on a change in pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85#discussion_r685865186
##########
File path: src/DotPulsar/Internal/Watchdog.cs
##########
@@ -0,0 +1,61 @@
+namespace DotPulsar.Internal
+{
+ using System;
+ using System.Collections.Generic;
+ using System.Text;
+ using System.Threading;
+
+ public class Watchdog: IDisposable
+ {
+ private CancellationTokenSource _cancellationTokenSource;
+ private readonly TimeSpan _timeout;
+ private readonly Timer _timer;
+
+ public Watchdog(TimeSpan timeout)
+ {
+ _timeout = timeout;
+ _timer = new Timer(OnTimeout);
+ _cancellationTokenSource = new CancellationTokenSource();
+ }
+
+ public CancellationToken CancellationToken
+ {
+ get => _cancellationTokenSource.Token;
+ }
+
+ public void GotMessage()
+ {
+ ResetTimer();
+ }
+
+ public void Enable()
+ {
+ if (_cancellationTokenSource.IsCancellationRequested)
+ _cancellationTokenSource = new CancellationTokenSource();
+ ResetTimer();
+ }
+
+ public void Disable()
+ {
+ _timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
Review comment:
> If dueTime is Timeout.Infinite, the callback method is never invoked; the timer is disabled, but can be re-enabled by calling Change and specifying a positive value for dueTime.
This is disabling the timer without disposing it.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] Flipbed commented on a change in pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
Flipbed commented on a change in pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85#discussion_r685876970
##########
File path: src/DotPulsar/Internal/Watchdog.cs
##########
@@ -0,0 +1,61 @@
+namespace DotPulsar.Internal
+{
+ using System;
+ using System.Collections.Generic;
+ using System.Text;
+ using System.Threading;
+
+ public class Watchdog: IDisposable
+ {
+ private CancellationTokenSource _cancellationTokenSource;
+ private readonly TimeSpan _timeout;
+ private readonly Timer _timer;
+
+ public Watchdog(TimeSpan timeout)
+ {
+ _timeout = timeout;
Review comment:
Maybe we should add a hint to adding a few seconds in the UseWatchdog method in that case?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] PetterIsberg closed pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
PetterIsberg closed pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] Flipbed commented on a change in pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
Flipbed commented on a change in pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85#discussion_r685858165
##########
File path: src/DotPulsar/Internal/Watchdog.cs
##########
@@ -0,0 +1,61 @@
+namespace DotPulsar.Internal
+{
+ using System;
+ using System.Collections.Generic;
+ using System.Text;
+ using System.Threading;
+
+ public class Watchdog: IDisposable
+ {
+ private CancellationTokenSource _cancellationTokenSource;
+ private readonly TimeSpan _timeout;
+ private readonly Timer _timer;
+
+ public Watchdog(TimeSpan timeout)
+ {
+ _timeout = timeout;
+ _timer = new Timer(OnTimeout);
+ _cancellationTokenSource = new CancellationTokenSource();
+ }
+
+ public CancellationToken CancellationToken
+ {
+ get => _cancellationTokenSource.Token;
+ }
+
+ public void GotMessage()
+ {
+ ResetTimer();
+ }
+
+ public void Enable()
+ {
+ if (_cancellationTokenSource.IsCancellationRequested)
+ _cancellationTokenSource = new CancellationTokenSource();
+ ResetTimer();
+ }
+
+ public void Disable()
+ {
+ _timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
Review comment:
Why not stop the timer instead of setting infinite timeout?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] Flipbed commented on a change in pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
Flipbed commented on a change in pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85#discussion_r685859578
##########
File path: src/DotPulsar/Abstractions/IPulsarClientBuilder.cs
##########
@@ -52,6 +52,11 @@ public interface IPulsarClientBuilder
/// </summary>
IPulsarClientBuilder RetryInterval(TimeSpan interval);
+ /// <summary>
+ /// Set a timeout for the watchdog to reconnect if no messages are received. The default disabled (using Timeout.InfiniteTimespan).
+ /// </summary>
+ public IPulsarClientBuilder UseWatchdog(TimeSpan timeout);
Review comment:
I think it should be enabled by default using the default timeout of 60 seconds. It's probably unwize to not use the watchdog.
##########
File path: src/DotPulsar/Internal/Watchdog.cs
##########
@@ -0,0 +1,61 @@
+namespace DotPulsar.Internal
+{
+ using System;
+ using System.Collections.Generic;
+ using System.Text;
+ using System.Threading;
+
+ public class Watchdog: IDisposable
+ {
+ private CancellationTokenSource _cancellationTokenSource;
+ private readonly TimeSpan _timeout;
+ private readonly Timer _timer;
+
+ public Watchdog(TimeSpan timeout)
+ {
+ _timeout = timeout;
Review comment:
The watchdog should probably add a few seconds (3 maybe?) to the timeout to compensate for any potential travel time delays between the server and client.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-dotpulsar] PetterIsberg commented on a change in pull request #85: Add a watchdog to the connections
Posted by GitBox <gi...@apache.org>.
PetterIsberg commented on a change in pull request #85:
URL: https://github.com/apache/pulsar-dotpulsar/pull/85#discussion_r685868329
##########
File path: src/DotPulsar/Internal/Watchdog.cs
##########
@@ -0,0 +1,61 @@
+namespace DotPulsar.Internal
+{
+ using System;
+ using System.Collections.Generic;
+ using System.Text;
+ using System.Threading;
+
+ public class Watchdog: IDisposable
+ {
+ private CancellationTokenSource _cancellationTokenSource;
+ private readonly TimeSpan _timeout;
+ private readonly Timer _timer;
+
+ public Watchdog(TimeSpan timeout)
+ {
+ _timeout = timeout;
Review comment:
I think it would be up to the client to set a timeout that takes server configuration, network travel time and packet loss into consideration. E.g. server ping time x2 or x3.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org