Skip to content

[doc] Update thread information in developer docs #18645

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Apr 15, 2020
Merged

[doc] Update thread information in developer docs #18645

maflcko merged 1 commit into from
Apr 15, 2020

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Apr 15, 2020

@fanquake fanquake added the Docs label Apr 15, 2020
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Apr 15, 2020

Thanks. Glad someone takes care of this. Such documentations do quickly outdate.
ACK 04c466aaf58cbbf984a37dafe40bcc634b460f37

@hebasto
Copy link
Member

hebasto commented Apr 15, 2020

Concept ACK.

Maybe add thread names that are available via tools like ps or top:
Screenshot from 2020-04-15 14-05-19
?

maflcko
maflcko reviewed Apr 15, 2020
View reviewed changes
@jnewbery jnewbery force-pushed the 2020-04-doc-threads branch from 04c466a to c91fba7 Compare April 15, 2020 13:50
@jnewbery
Copy link
Contributor Author

jnewbery commented Apr 15, 2020

Maybe add thread names that are available via tools like ps or top:

Done

What about adding links to the thread, so that when it is removed in the future, it will be immediately clear from the dead link?

Done

@jamesob
Copy link
Contributor

jamesob commented Apr 15, 2020

ACK c91fba7
Nice update!

maflcko
maflcko reviewed Apr 15, 2020
View reviewed changes
maflcko
maflcko reviewed Apr 15, 2020
View reviewed changes
maflcko
maflcko reviewed Apr 15, 2020
View reviewed changes
@maflcko
Copy link
Member

maflcko commented Apr 15, 2020

ACK c91fba7f60b129387fe7ccabb93f28601de80e68 , except for the wrong links

@jnewbery jnewbery force-pushed the 2020-04-doc-threads branch from 509ed8d to 66e67cf Compare April 15, 2020 14:57
@jnewbery
Copy link
Contributor Author

jnewbery commented Apr 15, 2020

Thanks @MarcoFalke. I've taken your changes.

@maflcko
Copy link
Member

maflcko commented Apr 15, 2020

ACK 66e67cfae16fd9154514b5d72888ceb49d52f390 only clicked the links, didn't read the comments

hebasto
hebasto reviewed Apr 15, 2020
View reviewed changes
hebasto
hebasto reviewed Apr 15, 2020
View reviewed changes
@jnewbery jnewbery force-pushed the 2020-04-doc-threads branch from 66e67cf to 808ef36 Compare April 15, 2020 16:02
@jnewbery
Copy link
Contributor Author

jnewbery commented Apr 15, 2020

Thanks for the review @hebasto . I've fixed your review comments.

hebasto
hebasto approved these changes Apr 15, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 808ef36.

@maflcko
Copy link
Member

maflcko commented Apr 15, 2020

ACK 808ef36

@maflcko maflcko merged commit e84a5f0 into bitcoin:master Apr 15, 2020
@jnewbery jnewbery deleted the 2020-04-doc-threads branch April 15, 2020 20:34
jonatack
jonatack reviewed Apr 15, 2020
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK. Nice idea to add the doxygen links.

doc/developer-notes.md

- ThreadRPCServer : Remote procedure call handler, listens on port 8332 for connections and services them.
- [ThreadMessageHandler (`b-msghand`)](https://doxygen.bitcoincore.org/class_c_connman.html#aacdbb7148575a31bb33bc345e2bf22a9)
: Application level message handling (sending and receiving). Almost
Copy link
Member

@jonatack jonatack Apr 15, 2020

Choose a reason for hiding this comment

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

s/Application level/Application-level/

doc/developer-notes.md
: Universal plug-and-play startup/shutdown.

- [ThreadSocketHandler (`b-net`)](https://doxygen.bitcoincore.org/class_c_connman.html#a765597cbfe99c083d8fa3d61bb464e34)
: Sends/Receives data from peers on port 8333.
Copy link
Member

@jonatack jonatack Apr 15, 2020

Choose a reason for hiding this comment

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

s/Receives/receives/

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 16, 2020
Merge bitcoin#18645: [doc] Update thread information in developer docs
5d8334f 
808ef36 [doc] Update thread information in developer docs (John Newbery)

Pull request description:

  - DumpAddresses thread was removed in bitcoin#5964
  - Shutdown thread was removed in bitcoin#5679
  - Add new threads (scheduler, RPC worker, indexer, tor control)
  - Small changes to documentation of other threads

ACKs for top commit:
  MarcoFalke:
    ACK 808ef36
  hebasto:
    ACK 808ef36.

Tree-SHA512: 85b6ace7bcc4dee030c63461bef1ded1a9581d4fa249c59f6fcd5d33d89c4357a6b8b35888ce0960f276d397b5e38a21e6c5d4b7b79544827a28c950e097b36d
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 15, 2021
@jnewbery @PiRK
[doc] Update thread information in developer docs
5a31841 
Summary:
This is a backport of Core [[bitcoin/bitcoin#18645 | PR18645]]

Note: the StartNode line was removed in [[bitcoin/bitcoin#17539 | PR17539]] (not yet backported)

Test Plan: Clicked on all the URLs to check that they work.

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8930
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

+2 more reviewers

@jonatack jonatack jonatack left review comments

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jnewbery @jonasschnelli @hebasto @jamesob @maflcko @jonatack @fanquake