Ulrich, Thanks for your comments. Please see inline:
On Mon, Apr 23, 2018 at 1:55 AM, Ulrich Windl < [email protected]> wrote: > >>> The Lee-Man <[email protected]> schrieb am 16.04.2018 um 23:57 > in > Nachricht <[email protected]>: > > On Monday, April 16, 2018 at 2:38:01 PM UTC-7, The Lee-Man wrote: > >> > >> On Friday, April 13, 2018 at 3:19:11 PM UTC-7, [email protected] > >> wrote: > >>> > >>> -retry: > >>> - rc = iscsid_exec_req(&req, &rsp, 0, ISCSID_REQ_TIMEOUT); > >>> /* > >>> - * handle race where iscsid proc is starting up while we are > >>> - * trying to connect. > >>> + * Need to handle race where iscsid proc is starting up while > we > >>> are > >>> + * trying to connect. Retry with exponential backoff, start > from > >>> 50 ms. > >>> */ > >>> - if (rc == ISCSI_ERR_ISCSID_NOTCONN && retries < 30) { > >>> - retries++; > >>> - sleep(1); > >>> - goto retry; > >>> - } else if (rc) > >>> - iscsi_err_print_msg(rc); > >>> + for (msec = 50; msec <= 15000; msec <<= 1) { > >>> > >> > >> Why are you using 50 -> 15000? Isn't the sequence then going to be: > >> -> 50 > >> -> 100 > >> -> 200 > >> -> 400 > >> -> 800 > >> -> 1600 (too large) > >> > >> So why not either go to 800 or to 1600? > >> > >> > > Doh! I am off by a factor of 10! > > > > -> 3200, 6400, 12800 > > I'm late in the discussion, but I'd vote for three things: > 1) Make the limits configurable (first delay, maximum wait duration) > Personally I am not a fan of introducing manual tunables, the existing implementation does not allow configurable max time either. I would rather not to change that. > 2) Change the algorithm not to specify the last delay being used, but the > total amount of waiting > Make the last delay time to be 1/2 of the expected total wait duration already implies the total waiting time, right? 3) Log if waiting for some longer time > Again, this was not in the old implementation and I am reluctant to add it. Thanks - Cathy > > I had implemented something like that years ago in Perl. Maybe to make it > clearer what I'm thinking of, here's a code snipplet: > > > $self->Log(0, 'D', $logger, "Waiting up to $limit seconds for > condition") > if ($verbosity > 1 && !$result); > no integer; > for (my $wait = $min_wait; !$result && $limit > 0; > $limit -= $wait, $wait *= 2) { > $wait = $limit > if ($wait > $limit); > $self->Log(1, 'D', $logger, > "Waiting $wait (of $limit remaining) seconds") > if ($verbosity > 0 && !$result); > # sleep $wait seconds to allow condition to become true > select(undef, undef, undef, $wait); > $result = $condition_r->($self); > } > $self->Log(1, 'D', $logger, "$condition_txt with $limit seconds > remaining") > if ($verbosity > 0 && $result); > > [...] > > Regards, > Ulrich > > > > > -- > You received this message because you are subscribed to the Google Groups > "open-iscsi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/open-iscsi. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
