Skip to content

Do not block indefinitely on the semaphore (backport #1654)#1689

Merged
lasarojc merged 3 commits intov0.37.xfrom
mergify/bp/v0.37.x/pr-1654
Nov 24, 2023
Merged

Do not block indefinitely on the semaphore (backport #1654)#1689
lasarojc merged 3 commits intov0.37.xfrom
mergify/bp/v0.37.x/pr-1654

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Nov 24, 2023

This is an automatic backport of pull request #1654 done by Mergify.
Cherry-pick of 2679498 has failed:

On branch mergify/bp/v0.37.x/pr-1654
Your branch is up to date with 'origin/v0.37.x'.

You are currently cherry-picking commit 2679498c9.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   mempool/v0/reactor.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   config/config.go
	both modified:   config/toml.go
	both modified:   test/e2e/pkg/manifest.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

* Do not block indefinitely on the semaphore

* Cancel the context, irrespective of the flow followed

* Makes the code more readable

* Improving comment

* make linter happy

* Updating comments to match

* Commenting out `select` and leaving it as TODO for when Contexts are more widely used

* Cleaned up comments

(cherry picked from commit 2679498)

# Conflicts:
#	config/config.go
#	config/toml.go
#	test/e2e/pkg/manifest.go
@mergify mergify bot requested a review from a team as a code owner November 24, 2023 10:44
@mergify mergify bot added the conflicts label Nov 24, 2023
lasarojc and others added 2 commits November 24, 2023 09:25
* Experimental - Reduce # of connections effectively used to gossip transactions out (#1558)

* maxpeers for mempool

* mempool: fix max_peers bcast routine active flag

* Use semaphore to limit concurrency

* Rename MaxPeers to MaxOutboundPeers

* Add max_outbound_peers to config toml template

* Rename in error message

* Renams the parameter to highlight its experimental nature. Extend the AddPeer method to return an error. Moves the semaphone to outside the broadcast routine

* reverting the addition of error to AddPeer. It fails if the context is done and handling this case will be done some other time, when an actual context is passed into acquire.

* reverting the addition of error to AddPeer. It fails if the context is done and handling this case will be done some other time, when an actual context is passed into acquire.

* Fixing lint issue

* renaming semaphore to something more meaningful

* make default value 0, which is the same as the current behavior. 10 is the recommended value.

* adding new flag to manifest.go

* Adding changelog

* Improve the description of the parameter in the generated config file.

* Add metric to track the current number of active connections.

* Change metric to gauge type and rename it.

* e2e: Allow disabling the PEX reactor on all nodes in the testnet

* Apply suggestions from code review

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Update config/config.go comment

* fix lint error

* Improve config description

* Rename metric (remove experimental prefix)

* Add unit test

* Improve unit test

* Update mempool/reactor.go comment

---------

Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: lasarojc <lasaro@informal.systems>
Co-authored-by: hvanz <hernan.vanzetto@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>

* Updating test file, leaving it broken for now

* mempool: Limit gossip connections to persistent and non-persistent peers (experimental) (#1584)

* Ignore persistent peers from limiting of outbound connections

* Update 1558-experimental-gossip-limiting.md

Update changeling

* Fix typo in mempool/metrics.go

* Use two independent configs and semaphores for persistent and non-persistent peers

* Forgot to rename in test

* Update metric description

* Rename semaphores

* Add comment to unit test

---------

Co-authored-by: hvanz <hernan.vanzetto@gmail.com>

* Reverting to old way of reporting errors

* Reverting change that shouldn't have been included in cherry-pick

* Reverting tests to use older functions

* fix rebase merge

---------

Co-authored-by: Adi Seredinschi <adizere@gmail.com>
Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: hvanz <hernan.vanzetto@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>
@lasarojc lasarojc merged commit 7dae514 into v0.37.x Nov 24, 2023
@lasarojc lasarojc deleted the mergify/bp/v0.37.x/pr-1654 branch November 24, 2023 12:50
g3ndo9kar9 pushed a commit to skip-mev/cometbft that referenced this pull request Jan 16, 2024
…ometbft#1689)

* Do not block indefinitely on the semaphore (cometbft#1654)

* Do not block indefinitely on the semaphore

* Cancel the context, irrespective of the flow followed

* Makes the code more readable

* Improving comment

* make linter happy

* Updating comments to match

* Commenting out `select` and leaving it as TODO for when Contexts are more widely used

* Cleaned up comments

(cherry picked from commit 2679498)

# Conflicts:
#	config/config.go
#	config/toml.go
#	test/e2e/pkg/manifest.go

* Backports cometbft#1558 and cometbft#1584 to 0.38.x (cometbft#1592)

* Experimental - Reduce # of connections effectively used to gossip transactions out (cometbft#1558)

* maxpeers for mempool

* mempool: fix max_peers bcast routine active flag

* Use semaphore to limit concurrency

* Rename MaxPeers to MaxOutboundPeers

* Add max_outbound_peers to config toml template

* Rename in error message

* Renams the parameter to highlight its experimental nature. Extend the AddPeer method to return an error. Moves the semaphone to outside the broadcast routine

* reverting the addition of error to AddPeer. It fails if the context is done and handling this case will be done some other time, when an actual context is passed into acquire.

* reverting the addition of error to AddPeer. It fails if the context is done and handling this case will be done some other time, when an actual context is passed into acquire.

* Fixing lint issue

* renaming semaphore to something more meaningful

* make default value 0, which is the same as the current behavior. 10 is the recommended value.

* adding new flag to manifest.go

* Adding changelog

* Improve the description of the parameter in the generated config file.

* Add metric to track the current number of active connections.

* Change metric to gauge type and rename it.

* e2e: Allow disabling the PEX reactor on all nodes in the testnet

* Apply suggestions from code review

Co-authored-by: Sergio Mena <sergio@informal.systems>

* Update config/config.go comment

* fix lint error

* Improve config description

* Rename metric (remove experimental prefix)

* Add unit test

* Improve unit test

* Update mempool/reactor.go comment

---------

Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: lasarojc <lasaro@informal.systems>
Co-authored-by: hvanz <hernan.vanzetto@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>

* Updating test file, leaving it broken for now

* mempool: Limit gossip connections to persistent and non-persistent peers (experimental) (cometbft#1584)

* Ignore persistent peers from limiting of outbound connections

* Update 1558-experimental-gossip-limiting.md

Update changeling

* Fix typo in mempool/metrics.go

* Use two independent configs and semaphores for persistent and non-persistent peers

* Forgot to rename in test

* Update metric description

* Rename semaphores

* Add comment to unit test

---------

Co-authored-by: hvanz <hernan.vanzetto@gmail.com>

* Reverting to old way of reporting errors

* Reverting change that shouldn't have been included in cherry-pick

* Reverting tests to use older functions

* fix rebase merge

---------

Co-authored-by: Adi Seredinschi <adizere@gmail.com>
Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: hvanz <hernan.vanzetto@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>

* Fixes conflict.

---------

Co-authored-by: lasaro <lasaro@informal.systems>
Co-authored-by: Adi Seredinschi <adizere@gmail.com>
Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Co-authored-by: Daniel Cason <daniel.cason@informal.systems>
Co-authored-by: hvanz <hernan.vanzetto@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
Co-authored-by: Sergio Mena <sergio@informal.systems>
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.

1 participant