[1.0.3 -> main] Test failure: p2p_sync_throttle_test

[greg7mdp]

Resolves #882.

While moving some code around (see here), a call to:

void connection::close(bool reconnect, bool shutdown)

became a call to:

int close(int __fd)

This restores the intended behavior (close the actual connection, instead of closing fd number 1)

[spoonincode]

obviously we need to merge this in now, but seems the tests are all failing

[heifner]

obviously we need to merge this in now, but seems the tests are all failing

Seems it has exposed a bug in sync wait timeout causing syncing to not continue correctly. I'm looking into it.

[greg7mdp]

Seems it has exposed a bug in sync wait timeout causing syncing to not continue correctly. I'm looking into it.

I wonder if sync_reassign_fetch:

spring/plugins/net_plugin/net_plugin.cpp

Lines 2190 to 2191 in c431b49

	sync_reassign_fetch( c );
	c->close(true);

)

calls request_next_chunk:

spring/plugins/net_plugin/net_plugin.cpp

Lines 2200 to 2212 in c431b49

	void sync_manager::sync_reassign_fetch(const connection_ptr& c) {
	fc::unique_lock g( sync_mtx );
	if( c == sync_source ) {
	peer_ilog(c, "reassign_fetch, our last req is ${cc}, next expected is ${ne}",
	("cc", sync_last_requested_num)("ne", sync_next_expected_num));
	c->cancel_sync();
	auto lib = my_impl->get_chain_lib_num();
	sync_last_requested_num = 0;
	sync_next_expected_num = std::max(sync_next_expected_num, lib + 1);
	sync_source.reset();
	request_next_chunk();
	}
	}

which reassigns the same connection to sync_source, which we close immediately afterwards?

[heifner]

which reassigns the same connection to sync_source, which we close immediately afterwards?

Yes, that is possible in general and since only one connection for this test then it would be the same one. However, I think the issue here is that the throttling node is not sending any blocks after it hits the throttle state.

[greg7mdp]

since only one connection for this test then it would be the same one

How can it not be a problem if we post a request and then immediately after close the connection?

[heifner]

How can it not be a problem if we post a request and then immediately after close the connection?

The connection has not sent a block in an expected timeframe. The response to that is to disconnect. That is our only connection for this test, but regardless, it is the right thing to do. The real bug here is that the node didn't send a block in the expected time frame, see #899.

If we close the connection then any pending requests are not sent.

[ericpassmore]

Note:start
category: System Stability
component: P2P
summary: Fix p2p sync to close connection.
Note:end