| Subject: | Retry broken |
While investigating bogus device unreachable errors due to missing ping responses I've discovered that the remaining retry counter is used as timeout and the retry counter is always set to 1 because of a missing timeout parameter:
line 677:
_do_ping($kernel, $heap, $sender, $event, $address, $remaining - 1, 1);
should be
_do_ping($kernel, $heap, $sender, $event, $address, $timeout $remaining - 1, 1);
Fixing this revealed an even more serious bug which seems to have been introduced by commit 27c7a7d where packets of a retry aren't handled correctly internally because $heap->{ping_by_seq}->{$seq} only stores the PBS_REQUEST_TIME and lacks all other fields.
https://github.com/rcaputo/poe-component-client-ping/commit/27c7a7dad945265b645d3eaef20016cad7414990#diff-dab1eed0a13a0b6f310ba0cad69ce9dfL405
My current patch doesn't handle retry requests differently than the initial one as a new $seq is allocated anyways and removes the $is_a_retry parameter of _do_ping completely:
diff --git a/lib/site_perl/5.22.1/POE/Component/Client/Ping.pm b/lib/site_perl/5.22.1/POE/Component/Client/Ping.pm
index 027e9ea..060cfaf 100644
--- a/lib/site_perl/5.22.1/POE/Component/Client/Ping.pm
+++ b/lib/site_perl/5.22.1/POE/Component/Client/Ping.pm
@@ -225,16 +225,14 @@ sub poco_ping_ping {
DEBUG and warn "ping requested for $address ($tries_left try/tries left)\n";
- _do_ping(
- $kernel, $heap, $sender, $event, $address, $timeout, $tries_left, 0
- );
+ _do_ping($kernel, $heap, $sender, $event, $address, $timeout, $tries_left);
}
sub _do_ping {
my (
- $kernel, $heap, $sender, $event, $address, $timeout, $tries_left,
- $is_a_retry
+ $kernel, $heap, $sender, $event, $address, $timeout, $tries_left
+ #$is_a_retry
) = @_;
# No current pings. Open a socket, or setup the existing one.
@@ -342,7 +340,7 @@ sub _do_ping {
$event, # PEND_EVENT
$address, # PEND_ADDR ???
$timeout, # PEND_TIMEOUT
- $is_a_retry, # PEND_IS_RETRY
+ #$is_a_retry, # PEND_IS_RETRY
];
if ($tries_left and $tries_left > 1) {
@@ -386,7 +384,7 @@ sub _send_next_packet {
$event, # PEND_EVENT
$address, # PEND_ADDR ???
$timeout, # PEND_TIMEOUT
- $is_a_retry, # PEND_IS_RETRY
+ #$is_a_retry, # PEND_IS_RETRY
) = @$ping_info;
# Send the packet. If send() fails, then we bail with an error.
@@ -421,12 +419,12 @@ sub _send_next_packet {
$kernel->delay( $seq => $timeout );
DEBUG_PBS and warn "recording ping_by_seq($seq)";
- if ($is_a_retry) {
+ #if ($is_a_retry) {
# If retries, set the request time to the new/actual request time.
# Inserted by Ralph Schmitt 2009-09-12.
- $heap->{ping_by_seq}->{$seq}->[PBS_REQUEST_TIME] = time();
- }
- else {
+ # $heap->{ping_by_seq}->{$seq}->[PBS_REQUEST_TIME] = time();
+ #}
+ #else {
$heap->{ping_by_seq}->{$seq} = [
# PBS_POSTBACK
$sender->postback(
@@ -440,7 +438,7 @@ sub _send_next_packet {
$address, # PBS_ADDRESS
time() # PBS_REQUEST_TIME
];
- }
+ #}
# Duplicate pings? Forcibly time out the previous one.
if (exists $heap->{addr_to_seq}->{$sender}->{$address}) {
@@ -674,7 +672,7 @@ sub poco_ping_default {
if ($retryinfo) {
my ($sender, $event, $address, $timeout, $remaining) = @{$retryinfo};
DEBUG and warn("retrying ping for $address (", $remaining - 1, ")\n");
- _do_ping($kernel, $heap, $sender, $event, $address, $remaining - 1, 1);
+ _do_ping($kernel, $heap, $sender, $event, $address, $timeout, $remaining - 1);
return;
}