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

Add ListenBrainz architecture docs #1826

Merged
merged 16 commits into from Feb 2, 2022
Merged

Add ListenBrainz architecture docs #1826

merged 16 commits into from Feb 2, 2022

Conversation

amCap1712
Copy link
Member

Initial draft of ListenBrainz architecture docs

@amCap1712 amCap1712 changed the base branch from master to amCap1712-patch-1 January 19, 2022 19:44
Copy link
Contributor

@MonkeyDo MonkeyDo 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 suggestions, feel free to merge suggestions or modify accordingly.

docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
Co-authored-by: Monkey Do <MonkeyDo@users.noreply.github.com>
===================

Services exclusive to ListenBrainz
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Collaborator

Choose a reason for hiding this comment

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

"This is a list of the docker containers for ListenBrainz services running in the MetaBrainz server infrastructure"

Copy link
Collaborator

Choose a reason for hiding this comment

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

is this list in any specific order? we could break it up by webservers, long-running services (ts writer, spot reader), utils (redis, exim), databases (timescale, typesense)

Almost all of these items relate to a specific service in docker-compose.yml, or a python entrypoint. They're also mostly listed in the docker/services directory. I wonder if we could link each item in this list to one of those. Does that make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the list to format as a table and made some groupings. If the current ones don't look good, I am fine with changing to a better grouping.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have linked the development services to docker-compose.yml. Not sure what we should link prod services to.

docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Outdated Show resolved Hide resolved
docs/developers/architecture.rst Show resolved Hide resolved
Comment on lines 68 to 71
On the other hand, "Permanent" Listens need to be persisted in the database. Timescale Writer service consumes from the
Incoming queue. It begins with querying the MessyBrainz database for MessyBrainz IDs. MessyBrainz tries to
find an existing match for the hash of the listen in the database. If one exists, it is returned otherwise it inserts
the hash and data into the database and returns a new MessyBrainz ID.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion:

Permanent Listens need to be persisted in the database. The Timescale Writer service consumes from the
Incoming queue. It queries the MessyBrainz database to get a temporary MessyBrainz ID for this Listen.
MessyBrainz tries to find an existing match based on a hash of the contents of the listen metadata.
If one exists, it is returned otherwise it inserts the hash and metadata into the database and returns a new MessyBrainz ID.

These services are also described in the "Production Services" section. Do you think it makes sense to talk about components using the name that they have in that list?

Copy link
Member Author

Choose a reason for hiding this comment

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

The names are quite similar so it is probably fine this way. Adding listenbrainz and prod repeatedly here seems distracting to me.

docs/maintainers/dumps.rst Outdated Show resolved Hide resolved
docs/maintainers/spotify-reader.rst Outdated Show resolved Hide resolved
docs/maintainers/dumps.rst Outdated Show resolved Hide resolved
docs/maintainers/dumps.rst Show resolved Hide resolved
docs/maintainers/dumps.rst Outdated Show resolved Hide resolved
@amCap1712 amCap1712 marked this pull request as ready for review January 26, 2022 15:22
Copy link
Collaborator

@alastair alastair left a comment

Choose a reason for hiding this comment

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

let's go ahead and merge this! we can make any further changes as we come across them

@alastair alastair merged commit f2cd1e8 into amCap1712-patch-1 Feb 2, 2022
@alastair alastair deleted the more-docs branch February 2, 2022 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants