client/authority-discovery: Publish and query on exponential interval

[mxinden]

When a node starts up publishing and querying might fail due to various
reasons, for example due to being not yet fully bootstrapped on the DHT.
Thus one should retry rather sooner than later. On the other hand, a
long running node is likely well connected and thus timely retries are
not needed. For this reasoning use an exponentially increasing interval
for publish_interval, query_interval and
priority_group_set_interval instead of a constant interval.

Fixes #7537.

polkadot companion: paritytech/polkadot#1953

@bkchr could you test this patch with your integration tests that depend
on the authority discovery module?

[mxinden]

Preparing Polkadot companion now.

[bkchr]

@bkchr could you test this patch with your integration tests that depend
on the authority discovery module?

The integration test is in master. You just need to switch to the adder-collator dir and run cargo test --release --features=real-overseer

[ordian]

@bkchr could you test this patch with your integration tests that depend
on the authority discovery module?

The integration test is in master. You just need to switch to the adder-collator dir and run cargo test --release --features=real-overseer

Speaking of, should we change

substrate/.maintain/gitlab/check_polkadot_companion_build.sh

Line 94 in 11ace4e

time cargo test --all --release --verbose --features=real-overseer

to the same as paritytech/polkadot#1949?

[mxinden]

@bkchr could you test this patch with your integration tests that depend
on the authority discovery module?

The integration test is in master. You just need to switch to the adder-collator dir and run cargo test --release --features=real-overseer

I am guessing that you are referring to polkadot/parachain/test-parachains/adder/collator, correct? Running cargo test -p test-parachain-adder-collator --release --features=real-overseer succeeds. More precisely a single test is executed, namely tests::collator_works, which succeeds. Let me know in case I am missing something.

[ordian]

@bkchr could you test this patch with your integration tests that depend
on the authority discovery module?

The integration test is in master. You just need to switch to the adder-collator dir and run cargo test --release --features=real-overseer

I am guessing that you are referring to polkadot/parachain/test-parachains/adder/collator, correct? Running cargo test -p test-parachain-adder-collator --release --features=real-overseer succeeds. More precisely a single test is executed, namely tests::collator_works, which succeeds. Let me know in case I am missing something.

Yes, I was referring to

cargo test --all --release --verbose --features=real-overseer

not running this test, so that we need to add another line to run it in companion check.

[mxinden]

@ordian @bkchr does this look good to you?

[bkchr]

Suggested change

	self.delay = Delay::new(self.next);
	self.delay = Delay::new(self.next);
	// Poll once to register the future in the current task.
	let _ = self.delay.poll_unpin(cx);

[tomaka]

Indeed this need to be done, otherwise the thing doesn't work at all.
However I'd suggest to put a loop around the entire body of poll_next instead of doing a dummy poll.

[mxinden]

Good catch @bkchr, thanks!

@tomaka can you expand on your idea of a loop around the entire body?

[tomaka]

Wait, no, you actually don't need this. (this code had passed my review the other day)

Since you return Poll::Ready, the caller has to call poll_next again, and this dummy registration isn't necessary.

To phrase it differently: the documentation of poll_next mentions that you pass a Waker that is woken up once the task is ready. In this case, you're returning Ready, so there's no need to register the Waker.

[bkchr]

Ahh yeah, I mixed Stream with Future. Sorry!

[mxinden]

For the record, this patch has been running on a Kusama validator since 2020-11-16. Below you can see the initial exponentially increasing refills of the pending lookups queue.

[ordian]

bot merge

[ghost]

Trying merge.