Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: reduce max utxo query size #3347

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Sep 15, 2021

Description

Reduced the wallet's max_utxo_query_size

Motivation and Context

Multiple IO Error: frame size too big. errors occurred in the RPC client with UTXO validation queries during console wallet and base node testing. This resulted in the RPC connection being closed and the UTXO validation task retrying the same thing.

**Edit: ** The error occurred in the wallet comms.

How Has This Been Tested?

System-level testing between the console wallet having a large wallet and the base node.

Reduced max_utxo_query_size due to `IO Error: frame size too big.` errors
in the RPC client during console wallet and base node testing.
@stringhandler
Copy link
Collaborator

Not sure is this is a bug? @sdbondi what do you think?

@sdbondi
Copy link
Member

sdbondi commented Sep 15, 2021

@mikethetike @hansieodendaal Almost certainly caused by some peers are running the old code before breaking changes in PR #3336 - including a reduction in frame size, which makes sense given this error message. Running a node without force_sync_peers set to check.

@stringhandler stringhandler added the P-do_not_merge Process - Not ready for merging label Sep 15, 2021
@sdbondi
Copy link
Member

sdbondi commented Sep 16, 2021

@hansieodendaal Have you tried setting your base node on the wallet to a base node running the latest code?

@hansieodendaal
Copy link
Contributor Author

@sdbondi, base node and wallet was on the latest code. The error occurred in the wallet comms due to queries from the console wallet to the base node that was too big, I guess probably due to changing transaction output data sizes.

We may need to investigate why reducing the batch size was now again necessary, as it was reduced not long ago? I do not recall recent changes in the data model.

@sdbondi
Copy link
Member

sdbondi commented Sep 20, 2021

@hansieodendaal ok I'll take a closer look at this and see where things need to be optimised

@sdbondi
Copy link
Member

sdbondi commented Sep 22, 2021

@hansieodendaal @mikethetike So 3500 x ~1000 bytes(depends on tariscript) = 3.5MB which is still in the 4MB limit. This method should be streaming to ensure that responses are within 4MB. Having said this, I cannot see why this needs to be reduced. I'll have to take a look at your logs.

Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this can go in, this RPC method needs to be streaming though. Less UTXOs in a batch means more request response rounds which means slower TX validation.

Testing locally I am not able to reproduce any issues with 3500, please feel free to message me with logs (making sure that both sides support chunking) and I'll see if I can find anything.

@stringhandler stringhandler added mq-approved and removed P-do_not_merge Process - Not ready for merging labels Sep 23, 2021
@aviator-app aviator-app bot merged commit 1993a7e into tari-project:development Sep 23, 2021
@hansieodendaal hansieodendaal deleted the ho_max_utxo_query_size branch October 14, 2021 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants