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

Configurable size of validate queue #255

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

pdyraga
Copy link
Contributor

@pdyraga pdyraga commented Jan 21, 2020

validateWorker() reads from validateQ and invokes validate function that performs validation of the message. Signature validation is performed synchronously. The number of validate workers defaults to the number of CPUs and can be updated with WithValidateWorkers function. With no additional user validators, signature validation is the bottleneck when receiving new messages.

Increasing the number of validating workers does not help given the context switching and bottleneck nature of this spot. As stated in WithValidateWorkers documentation, this function should be used rather to limit the number of workers to devote less CPU time for synchronous validation. On the other hand, with the default size of validateQ, some applications built on a top of libp2p may experience throttled validation and lost messages.

This problem is addressed in this PR with WithValidateQueueSize allowing to configure the buffer size for synchronous validation. Application developers knowing the nature of their protocols can set this value to minimise the possibility of throttled synchronous validation and dropped messages. Configurable buffer size allows to gracefully handle peaks of messages and, from the other side, the number of concurrent synchronous workers is still limited by validateWorkers property so the receiver should not get congested.

validateWorker() reads from validateQ and invokes validate function
that performs validation of the message. Signature validation is performed
synchronously. The number of validate workers defaults to the number of CPUs
and can be updated with WithValidateWorkers function. With no additional user
validators, signature validation is the bottleneck when receiving new messages.

Increasing the number of validating workers does not help given the context
switching and bottleneck nature of this spot. As stated in WithValidateWorkers
documentation, this function should be used rather to limit the number of workers
to devote less CPU time for synchronous validation. On the other hand, with the
default size of `validateQ`, some applications built on a top of libp2p may
experience throttled validation and lost messages.

This problem is addressed by WithValidateQueueSize allowing to configure the buffer
size for synchronous validation. Application developers knowing the nature of their
protocols can set this value to minimise the possibility of throttled synchronous
validation and dropped messages. Configurable buffer size allows to gracefully
handle peaks of messages and, from the other side, the number of concurrent
synchronous workers is still limited by validateWorkers property so the receiver
should not get congested.
@vyzo
Copy link
Collaborator

vyzo commented Jan 27, 2020

Can you go fmt please? Travis rejects it for formatting issues.

@pdyraga
Copy link
Contributor Author

pdyraga commented Jan 27, 2020

Ready! I see the build is failing but I think it could be some timeout. Unfortunately, I don't see an option to rerun it.

Copy link
Collaborator

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

thank you!

@vyzo vyzo merged commit 25c434f into libp2p:master Jan 27, 2020
pdyraga added a commit to keep-network/keep-core that referenced this pull request Jan 31, 2020
We had to switch to our fork because of the PR we submitted to libp2p
allowing to configure size of validation queue with
WithValidateQueueSize function.

libp2p/go-libp2p-pubsub#255

This PR is merged so we can go back to the libp2p/go-libp2p-pubsub.
Since there was no official release from the time our PR was merged, we
are attached to a commit in go-libp2p-pubsub master.
lukasz-zimnoch added a commit to keep-network/keep-core that referenced this pull request Jan 31, 2020
Switch back to libp2p/go-libp2p-pubsub

We had to switch to our fork because of the PR we submitted to libp2p allowing to configure size of validation queue with WithValidateQueueSize function:

libp2p/go-libp2p-pubsub#255

This PR is merged so we can go back to the libp2p/go-libp2p-pubsub. Since there was no official release from the time our PR was merged, we are attached to a commit in go-libp2p-pubsub master.
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.

2 participants