Skip to content

fix(e2e): fix p2p deadlock#473

Merged
shotonoff merged 14 commits intov0.10-devfrom
sbe-fix-e2e-tests
Oct 20, 2022
Merged

fix(e2e): fix p2p deadlock#473
shotonoff merged 14 commits intov0.10-devfrom
sbe-fix-e2e-tests

Conversation

@shotonoff
Copy link
Collaborator

@shotonoff shotonoff commented Oct 6, 2022

Issue being fixed or feature implemented

Despite the dashcore e2e network configuration being stable, the rotate network configuration does not work, it needs to find out the root cause and fix it.

What was done?

  • Identified the root cause of deadlock: runWithPeerMutex is acquired a lock that is redundant and not necessary, because it doesn't modify a p2p router state. removing this method eliminates the deadlock.
  • added checking a snapshot file on existence in abci/example/kvstore/snapshots.go
  • simplified kvstore.Application.getValidatorSetUpdate
  • removed conditions using config.P2P.PexReactor flag to use p2p PEX reactor. it is not required, because tenderdash supports only one implementation pex reactor.

How Has This Been Tested?

Unit tests / E2E tests

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

}
}

func (s *offerSnapshot) reset() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

func (*offerSnapshot).reset is unused (unused)

nodeMetrics.consensus.BlockSyncing.Set(1)
}

if cfg.P2P.PexReactor {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this condition was removed?

If it's by purpose, did you also remove corresponding config code and config example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we don't have other behaviour, PEX must be only P2P.
regarding your question: I decided to separate these changes. next changes should remove this parameter from config.

@lklimek
Copy link
Collaborator

lklimek commented Oct 20, 2022

Please change PR title to sth that will go into changelog.

@shotonoff shotonoff changed the title fix(e2e): sbe fix e2e tests fix(e2e): fix p2p deadlock Oct 20, 2022
Copy link
Collaborator Author

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

done

nodeMetrics.consensus.BlockSyncing.Set(1)
}

if cfg.P2P.PexReactor {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we don't have other behaviour, PEX must be only P2P.
regarding your question: I decided to separate these changes. next changes should remove this parameter from config.

@shotonoff shotonoff requested a review from lklimek October 20, 2022 15:06
@shotonoff shotonoff merged commit bd91a85 into v0.10-dev Oct 20, 2022
@lklimek lklimek deleted the sbe-fix-e2e-tests branch February 1, 2024 13:33
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