Notifications - Model, implementation and tests.

[raduom]

Pre-submit checklist:

• Branch
  • [x] Tests are provided (if possible)
  • [x] Commit sequence broadly makes sense
  • [x] Key commits have useful messages
  • [x] Formatting, PNG optimization, etc. are updated
  • [x] Important changes are reflected in changelog.d of the affected packages
• PR
  • [x] Self-reviewed the diff
  • [x] Useful pull request description
  • [x] Reference the ADR in the PR and reference the PR in the ADR (if revelant)
  • [x] Reviewer requested

[raduom]

The difference between Marconi.Core.Storable and Marconi.Core.TracedStorable is very small, why not instead of adding a new file every time where it's hard to tell what changed and what stayed the same why not evolve the indexer API that we already have with added tracing and removed query interval? Needing to change all the indexer implementations would also show what are you meaning to do with this change. As far as I can tell, the current file removes interval and adds tracing while keeping the rest the same.

I made a new file because I will probably not have the time to also rewrite the indexers that use the new API. The way the indexer is right now it does not actually change the API, but I think that in it's final form we will have to do that. I will do some more work. It may be the case that changing the existing indexer to conform to the new API will be a very easy exercise.

Second question about using tracing for notifications: in the rest of our code bases tracing is used for logging, so how would an indexer user use it for getting the latest chain point processed, would they need to listen on the indexer's stdout and parse the output?

Traces can be emitted using any kind of contra-tracer adapter or even iohk-monitoring. One such adapter, for example, could open a websocket and publish updates as the node consumes blocks. Or, simpler, a TChan where these updates are forwarded. Some other software component listening to the TChans created by all indexers will have an accurate view of synchronisation status across all indexers.

If our query is built with JSON RPC then why not implement querying the latest-chainpoint-processed through it as well?

It's rather inconvenient for the user. I think this change is functionally the same, but easier for our users (we can provide the component that listens on the TChans and the user does not have to worry about it). It also enables the implementation of further functionalities in indexers.

[koslambrou]

@kayvank Can you change the ADR name :D

[berewt]

Unfortunately my last comment will be unanswered. I still think we may need an error handling if we query the indexer past its latest sync point. It can be updated when we'll start integrating the ADR in our codebase though.

[kayvank]

Discussed this with @berewt in detail. See the JIRA ticket @berewt created capturing the final approach/proposal for handling indexer-queries that areb past the latest sync point

[kayvank]

@kayvank Can you change the ADR name :D

how about: "Indexers query synchronisation primitive"?

[koslambrou]

how about: "Indexers query synchronisation primitive"?

@kayvank LGTM :). The file name and the title need to changed.