bisq

bisq copied to clipboard

Initial testing shows that a flag (initialRequestApplied) is preventing all subsequent data from being applied, resulting in 10 request/response of the same data set before giving up. Maybe that flag is not necessary any more.

https://github.com/bisq-network/bisq/blob/babdc750ac696f1b222be172b968aed40224b95f/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java#L514-L530

Sep 04 '22 16:09 ghost

@jmacxx Ah thanks for spotting that. Would need to look into it in more details. I guess we need to set the flag only once there is no truncation or we reached the max requests. It might require a bit more of refactoring as the 2 aspects are handled in 2 diff. areas. I will look into it in the next days, but can take a bit until I find time.

Sep 04 '22 18:09 chimp1984

My assumption for the various upgrade scenarios:

• New seednode, new client -> client requests again when isTruncated is set, until full data set is received.
• New seednode, old client -> Seednode sends new flag which client ignores. Clients would not make requests for chunked data downloads, stopping at the first 5k items delivered. Thus, old clients not connected in a while (4 weeks perhaps) would experience degraded data sets (5k rather than 10k items).
• Old seednode, new client -> This is not a likely scenario as we're going to upgrade the seednodes in advance of the clients. Client expects new flag which would not be present (error, need to test?)

Sep 28 '22 12:09 ghost

* **New seednode, old client** -> Seednode sends new flag which client ignores.  Clients would not make requests for chunked data downloads, stopping at the first 5k items delivered.  Thus, old clients not connected in a while (4 weeks perhaps) would experience degraded data sets (5k rather than 10k items).

Yes you are correct but I guess the next release will be a forced update as there are other important changes, so after a few days when about 80-90% have updated anyway we will enforce for trading the new version, thus should make that degradation case irrelevant.

Sep 28 '22 14:09 chimp1984

Tradestatistics display remains stuck with only the initial chunk until after restart. Maybe this is acceptable though.

Sep 30 '22 04:09 ghost

I'll merge this PR directly into the release branch to prevent any issue on master for those running it.

Oct 04 '22 07:10 ripcurlx