feat: adds persistence layer for app state and job results [SBK-363] #45

mikeshultz · 2023-11-18T02:28:55Z

What I did

Adds persistence for app state and job results. Alters startup/shutdown handlers a bit.

Closes #33

How I did it

Adds persistent storage support of state and results. Stores block numbers the runner sees in an app state collection. Stores handler job results in result collection.

How to verify it

Run it with these defined:

SILVERBACK_PERSISTENCE_CLASS="silverback.persistence:SQLitePersistentStorage"
SQLITE_PATH="/tmp/silverback.db"

Checklist

Passes all linting checks (pre-commit and CI jobs)
New test cases have been added and are passing
Documentation has been updated
PR title follows Conventional Commit standard (will be automatically included in the changelog)

linear · 2023-11-18T02:29:02Z

SBK-363 "feat: adds persistence layer for app state and job results" (ApeWorX/silverback #45)

WIP: This is still pretty raw, looking for early feedback. Will leave some personal notes inline.

What I did

Adds persistence for app state and job results

Closes #33

How I did it

Adds mongo support by way of beanie. Stores block numbers the runner sees in an app state collection. Stores handler job results in result collection.

How to verify it

Run it with SILVERBACK_MONGO_URI defined.

Checklist

Passes all linting checks (pre-commit and CI jobs)
New test cases have been added and are passing
Documentation has been updated
PR title follows Conventional Commit standard (will be automatically included in the changelog)

ApeWorX/silverback #45 by mikeshultz on GitHub

via LinearSync

silverback/application.py

mikeshultz · 2023-11-18T02:47:14Z

silverback/persistence.py

+        )
+
+
+class BasePersistentStorage(ABC):


Is there anything we need to add for v1? This is bare minimum for what this PR needs but we could future-think here as well for aggregate data or API fetching.

silverback/persistence.py

silverback/runner.py

mikeshultz · 2023-11-18T02:57:21Z

Here's what the results docs look like in MongoDB:

[
  {
    _id: ObjectId("65581e0f5a9cc7756a43e890"),
    instance: 'default',
    network: 'ethereum:mainnet',
    handler_id: 'event/0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48/Transfer(address,address,uint256)',
    block_number: 18595697,
    log_index: 222,
    execution_time: 0,
    return_value: Binary.createFromBase64("gASVMAAAAAAAAABDLIAElSEAAAAAAAAAQx2ABJUSAAAAAAAAAH2UjAZhbW91bnSUSjDZqBxzLpQulC4=", 0),
    created: ISODate("2023-11-18T02:14:39.684Z")
  },
  {
    _id: ObjectId("65581e0f5a9cc7756a43e891"),
    instance: 'default',
    network: 'ethereum:mainnet',
    handler_id: 'block/18595697',
    block_number: 18595697,
    log_index: null,
    execution_time: 0.57,
    return_value: Binary.createFromBase64("gASVGAAAAAAAAABDFIAElQkAAAAAAAAAQwWABEtlLpQulC4=", 0),
    created: ISODate("2023-11-18T02:14:39.841Z")
  }
]

And app state:

[
  {
    _id: ObjectId("65581e0f5a9cc7756a43e88e"),
    instance: 'default',
    network: 'ethereum:mainnet',
    last_block_seen: 18595697,
    last_block_processed: 18595697,
    updated: ISODate("2023-11-18T02:14:39.475Z")
  }
]

silverback/runner.py

…_* decorators

silverback/application.py

silverback/persistence.py

… network settings

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

docs/userguides/development.md

silverback/application.py

fubuloubu · 2023-11-27T22:25:51Z

silverback/application.py

+            def do_something_on_startup(startup_state):
+                ...  # Reprocess missed events or blocks
+        """
+        return self.broker.task(task_name="silverback_startup")


what are the pros/cons of moving the startup/shutdown handlers into separate tasks from the worker startup/shutdown events

They cannot be combined. They're very separate things.

Having on_startup be a worker event means unexpected duplication of execution. Having a separate task also allows us to feed the handler useful information, like last_block_*. This is basically what we need for ApePay.

Renaming the previous handlers to be explicit worker event handlers implies their actual usage and is clearer, IMO. They're two distinct things.

The only real con is we're changing up behavior here. Anyone setting up worker state may need to refactor.

They cannot be combined. They're very separate things.

Having on_startup be a worker event means unexpected duplication of execution. Having a separate task also allows us to feed the handler useful information, like last_block_*. This is basically what we need for ApePay.

Renaming the previous handlers to be explicit worker event handlers implies their actual usage and is clearer, IMO. They're two distinct things.

The only real con is we're changing up behavior here. Anyone setting up worker state may need to refactor.

but won't this mean that only one worker will handle this event on startup? vs. the worker startup event is executed on every worker as it's set up?

yes. on_startup will only run once upon runner startup. on_worker_startup will run on every worker startup.

silverback/middlewares.py

examples/redis/main.py

examples/memory/main.py

silverback/types.py

silverback/persistence.py

… feat/persistence

docs/userguides/development.md

fubuloubu · 2023-11-28T22:53:43Z

docs/userguides/development.md

+
+silverback run "example:app" \
+    --network :mainnet:alchemy \
+    --runner "silverback.runner:WebsocketRunner"


question(non-blocking): should we standardize runner to client?

Hard question, tbh. Would it be right to say "choose your client" to the user who wants to say use the WebsocketRunner?

docs/userguides/development.md

fubuloubu · 2023-11-28T23:02:53Z

silverback/middlewares.py

+        )
+
+        try:
+            await self.persistence.add_result(handler_result)


random question: does the middleware act on the client or the worker?

Both. pre_send and post_send are client side, and pre_execute, post_execute, and post_save are worker-side. It never comes back to the client after processing, though.

fubuloubu

suggestion: we find a different name rather than persistence as it's a little awkward and not a good indicator of it's purpose

maybe data_store or metrics

fubuloubu · 2023-11-28T23:08:06Z

silverback/settings.py

@@ -17,6 +18,9 @@ class Settings(BaseSettings, ManagerAccessMixin):
    testing or deployment purposes. Defaults to a working in-memory broker.
    """

+    # A unique identifier for this silverback instance
+    INSTANCE: str = "default"


What's the idea behind this?

The ability for multiple Silverback instances to use a single persistence backend. For ApePay, I'm setting it to apepay-sepolia, and we may have others for other networks. Otherwise, they'd step on each-others toes and confusingly share state.

silverback/types.py

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

mikeshultz · 2023-11-29T00:05:31Z

suggestion: we find a different name rather than persistence as it's a little awkward and not a good indicator of it's purpose

maybe data_store or metrics

I don't know. data_store is also clumsy since it's multiple words, and metrics doesn't really cover its use. Could just call it "store", "storage", or "state". You tell me.

mikeshultz added 3 commits November 17, 2023 18:27

feat: adds persistence layer for app state and job results

c46ec08

style(lint): fix imports

b956d48

fix: fix types

7ba2f71

vany365 changed the title ~~feat: adds persistence layer for app state and job results~~ feat: adds persistence layer for app state and job results [SBK-363] Nov 18, 2023

mikeshultz self-assigned this Nov 18, 2023

mikeshultz added the enhancement New feature or request label Nov 18, 2023

style(lint): disagreement flake8 didn't like black output

85a5579