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

Bolt12/p2p master attenuation #3281

Merged
merged 19 commits into from
Oct 8, 2021
Merged

Conversation

bolt12
Copy link
Contributor

@bolt12 bolt12 commented Jul 29, 2021

Adds Attenuation to tests and fixes a couple of bugs found in the way.

@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from 3895e0a to ffe1561 Compare August 12, 2021 13:41
@bolt12 bolt12 marked this pull request as ready for review August 12, 2021 15:01
@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from ffe1561 to 5ceb375 Compare August 12, 2021 16:22
@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch 4 times, most recently from 860f413 to 910b07d Compare September 6, 2021 12:51
@coot
Copy link
Contributor

coot commented Sep 6, 2021

Could you rebase the branch so that fixup! server-test: use DataFlowProtocolData is applied.

@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from 910b07d to 15f95a5 Compare September 6, 2021 12:59
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

This is quite large PR, we should avoid this in the future (reviewing is quite hard of such large changes). If needed an issue can always be broken into multiple PRs.

@bolt12 bolt12 requested a review from coot October 4, 2021 09:47
@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from c6bef14 to 8cbe8ed Compare October 4, 2021 15:25
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

A few more comments.

bolt12 and others added 13 commits October 7, 2021 14:21
When requestOutboundConnection is run we have two calls which needs
to take care of resources

* connect
* connection handler thread

This patch avoids having resource cleanups for both calls to overlap.
Add corner case transitions to verifyAbstractTrans

connection mngr: refactor
- Fixes cleanup function TerminatingSt case
- Adds and logs TrUnexpectedlyMissingConnectionState

Fixes DColdNoop reflexive transition tracing

Add TODO to Snocket.hs
The issue was noTimeoutHandshake was being used and there was a thread
 hanging forever waiting on timeout.

Refactor handshakeTimeLimits to top-level:

- This allows to decide when to pass timeLimitsHandhsake or
  noTimeLimitsHandshake, depending on the type of test we're running

Extended Server2 with Shutdown action

Make connectionLoop resilient so that it can't die
When closing a socket, it does not throw timeout exceptions.   If the
remote host does not reply, the connection will be evenautlly closed by
the OS.
This patch providess UnversionedProtocol which allows to negotiate
'DataFlow'.
When connection is negotiated, it can fail.  Pattern match on the
current state to verify that it's not 'TerminatedState'.
Since we start using non-trivial attenuation, we can expect IO
/ multiplexer errors.  They should not terminate a simulation.
This is not perfect, so we also remove an assertion from
connection-manager.  This assertion is benign and also hit rarely (once
per 100_000 simulations).
caused by a race with withConnectionManager. So ConnectionManager simulation had PopScheduleOutboundError and
leaked connections problems.

Both problems sources was due to race conditions caused by a wrong
assumption that the connVar could be reused if in the Terminating or
TerminatedState.

We also log TerminatingSt->TerminatedSt in the right place

Authors: Marcin Szamotulski
@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from 8cbe8ed to d312b0a Compare October 7, 2021 13:39
bolt12 and others added 6 commits October 7, 2021 15:24
Be careful when overwriting the state.
The resource handler for `bracketOnError` must be uninterruptible.
This simulation is not designed to handle errors.
These identity transitions are harmless, triggered by hot miniprotocol
termination.

Rebase fixs
@bolt12 bolt12 force-pushed the bolt12/p2p-master-attenuation branch from d312b0a to 483b2f3 Compare October 7, 2021 14:24
@bolt12 bolt12 requested a review from coot October 8, 2021 10:32
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@bolt12 bolt12 merged commit 66899fc into p2p-master Oct 8, 2021
@iohk-bors iohk-bors bot deleted the bolt12/p2p-master-attenuation branch October 8, 2021 12:08
@coot coot mentioned this pull request Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants