Details

Reviewers

olasd
zack

Group Reviewers

Reviewers

Commits

rDJNL0f7ccbb64e16: Add option to allow consumer client to choose fetch policy
rDJNL3831d5f72f9d: swh.journal.client: Ensure options are correctly set when starting
rDJNL4b45b74a975e: doc: Improve wording about SWHJournalClient class
rDJNLd21987820959: doc: Improve docstring and default sample
rDJNLc3f2cc60ac74: requirements: Add kafka dependency
rDJNL976130aaafa0: publisher: Configure the threshold number of messages to send
rDJNL389a9a34f18f: Add a journal client base class to process messages

Summary

This is a first draft to implement a base client from our swh-journal.

This can be used for example to trigger the creation of new content in
the archiver db.

Related T424
Related T494

Outside of that scope, i see some possible refactoring between the client class (a consumer) and the publisher (which is also a consumer).
Also between the publisher and the swh.storage.listener (the listener is a publisher).

Diff Detail

Repository

rDJNL Journal infrastructure

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Feb 25 2017, 1:16 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 25 2017, 1:16 AM

ardumont mentioned this in D181: Add journal client implementation which updates content archiver db with new contents.Feb 25 2017, 1:30 AM

ardumont edited the summary of this revision. (Show Details)Feb 25 2017, 1:34 AM

ardumont edited the summary of this revision. (Show Details)

doc: Improve docstring and default sample

ardumont mentioned this in D182: Deploy journal_publisher and archiver_content_updater manifest.Mar 1 2017, 11:15 AM

ardumont mentioned this in D183: Add journal_publisher and archiver_content_updater's configuration.

I agree with the general goal of consistency between the producer and consumer configuration options.

However, I think we should clearly separate both and making one a subclass of the other sounds like a recipe for confusion.

Furthermore, we will need two producers: the "live" producer subscribing to database events, as well as the "catchup" producer which will have a feedback loop between the journal and the database. I think there's more value in providing a base class for both of those rather than for producer and consumer.

swh/journal/client.py

17–34

The topic wording is an internal kafka thing that doesn't really matter (except as an implementation detail) for swh.journal consumers.

Proposed wording:

A base client for the Software Heritage journal.

The current implementation of the journal uses Apache Kafka brokers to publish messages under a given topic prefix, with each object type using a specific topic under that prefix.

Clients subscribe to events specific to each object type by using the `object_types` configuration variable.

Clients can be sharded by setting the `client_id` to a common value across instances. The journal will share the message throughput across the nodes sharing the same client_id.

Messages are processed by the `process_objects` method in batches of maximum `max_messages`.

That variable should be made configurable: we should allow consumer to only care about live events without a (big) catchup phase.

I agree with the general goal of consistency between the producer and consumer configuration options.

Nice.

However, I think we should clearly separate both and making one a subclass of the other sounds like a recipe for confusion.

Ok, that sounds fair.

Furthermore, we will need two producers: the "live" producer subscribing to database events, as well as the "catchup" producer which will have a feedback loop between the journal and the database.

I believe what you mention refers to:

the live producer is the actual listener defined in swh.storage.listener (subscribed to db events and feeding new object events to the publisher's journal).
the catchup producer being the checker mentioned in T494 (which will be 'diffing' the journal against the db and effectively provide back the missing content ids to the journal).

I think there's more value in providing a base class for both of those rather than for producer and consumer.

Ok.

Adaptation according to proposed improvments

doc: Improve wording about SWHJournalClient class
Add option to allow consumer client to choose fetch policy

ardumont marked 2 inline comments as done.Mar 2 2017, 4:45 PM

A few more nits but this looks reasonable to build upon.

swh/journal/client.py
40	Should be renamed to match the docstring (or the docstring changed to match)
63	Let's give a proper error message (with a ValueError). While we're checking the configuration, let's hard-code the list of supported object types and check that the configuration matches (again, raising ValueError on unknown values).

olasd accepted this revision.Mar 2 2017, 6:02 PM

olasd edited edge metadata.

This revision is now accepted and ready to land.Mar 2 2017, 6:02 PM

swh.journal.client: Ensure options are correctly set when starting

ardumont marked an inline comment as done.Mar 2 2017, 6:37 PM

Closed by commit rDJNL389a9a34f18f: Add a journal client base class to process messages (authored by ardumont). · Explain WhyMar 13 2017, 11:17 AM

This revision was automatically updated to reflect the committed changes.

Add a journal client base class to process messages
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 592

swh/journal/client.py

Add a journal client base class to process messagesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 592

swh/journal/client.py

Add a journal client base class to process messages
ClosedPublic
Actions

Revision Contents
Changeset List