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

Tracing improvements #2924

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Tracing improvements #2924

merged 2 commits into from
Feb 5, 2021

Conversation

coot
Copy link
Contributor

@coot coot commented Feb 4, 2021

  • mini-protocols: trace termination message
  • tx-submission-inbound: improved logging

@coot coot added chain-sync networking tx-submission Issues related to tx-submission protocol labels Feb 4, 2021
@coot coot requested review from dcoutts and karknu February 4, 2021 17:39
@coot coot changed the title coot/p2p base 2 Tracing improvements Feb 4, 2021
Copy link
Contributor

@karknu karknu left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -39,8 +40,7 @@ directPipelined (TxSubmissionServerPipelined mserver)
SendMsgReplyTxIds (BlockingReply txids) client' -> do
server' <- serverNext txids
directSender q server' client'
SendMsgDone b ->
return (a, b)
SendMsgDone b -> (,b) <$> a
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because a :: m a, while it used to be a :: a; using monadic action allows to log a message when sending MsgDone.

@coot
Copy link
Contributor Author

coot commented Feb 5, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 5, 2021

@iohk-bors iohk-bors bot merged commit 1f667af into master Feb 5, 2021
@iohk-bors iohk-bors bot deleted the coot/p2p-base-2 branch February 5, 2021 12:26
coot added a commit that referenced this pull request May 16, 2022
2876: DNS subscription refactor for A/AAAA lookup r=coot a=Infinisil

While looking into that code I noticed that I could be improved a bit. This includes:
- Removing the need for any extra `TMVar`'s and `TVar`'s
- Previously only exceptions for the first thread to finish were caught and logged. Now exceptions for both threads are caught
- Simplify the code for more readability

Other than all thread exceptions now being caught, this is only a refactoring without any functional change. To convince myself of the correctness of this change, I split the changes into smallish commits, which should relatively easily be verifiable for equivalence.

2924: Tracing improvements r=coot a=coot

- mini-protocols: trace termination message
- tx-submission-inbound: improved logging


Co-authored-by: Silvan Mosberger <contact@infinisil.com>
Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chain-sync tx-submission Issues related to tx-submission protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants