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

Events: System wide event queue to replace current broadcast. #630

Closed
jsimnz opened this issue Jul 13, 2022 · 7 comments
Closed

Events: System wide event queue to replace current broadcast. #630

jsimnz opened this issue Jul 13, 2022 · 7 comments
Assignees
Labels
area/db-system Related to the core system related components of the DB feature New feature or request

Comments

@jsimnz
Copy link
Member

jsimnz commented Jul 13, 2022

Part of #822

Currently we have a p2p-specific implementation for the broadcaster to announce new CRDT log events so the p2p system can publish the update over the network.

This can be generalized to work over the entire DB system, and include all kinds of events beyond new CRDT logs. Eg. More specific updates, create-log, update-log, create-collection, create-index, index-online (index is ready to be used and has been backfilled/updated to current state), new-peer, etc.

Moreover, this needs to concretely fix #629 so there is no race condition between the broadcaster and the consumer of the event queue.

This is a result of how the transactions work due to our ACID requirements. Data isn't officially in the DB until the transaction as been succesfully commited. However, the current design initiates the broadcast before the transaction is commited, and the consumer likely doesn't have access to the transaction so they access the DB directly to get the infromation required. But, the tx hasn't commited, so the data associated with the event doesn't yet exist as commited information on the DB.

The likely way out of this is to rely on a system already present in Defra, which is Transaction Callbacks (error & success). Which allow us to register functions to be executed on succesful commits or errors of a transaction. So we can link our event queue publish action to the tx success callback.

Something to explore.

@jsimnz jsimnz added area/db-system Related to the core system related components of the DB feature New feature or request labels Jul 13, 2022
@jsimnz jsimnz added this to the DefraDB v0.4 milestone Jul 13, 2022
@jsimnz
Copy link
Member Author

jsimnz commented Aug 5, 2022

Look into unifying with libp2p.Host.EventBus

@orpheuslummis
Copy link
Contributor

orpheuslummis commented Aug 5, 2022

problems to avoid:

  • over-serialization & blocking
  • backpressure from the event system badly impact other parts, e.g. activity in p2p affecting db

@AndrewSisley
Copy link
Contributor

Would be nice if this can also be used to remove the P2P/broadcasting stuff from the db package too IMO

@AndrewSisley
Copy link
Contributor

AndrewSisley commented Sep 19, 2022

Thought: We might also want to consider the following:

  • Persistence, should the event queue be wiped if the database goes offline? (should probs be configurable)
  • Persistence part 2, the capacity of the queue and how long the items remain in it should probably be configurable.
  • Exposure, it could be really handy to have the queue exposed as part of our public abi
  • Security/privacy - for the future, all items in the queue should probably not be exposed to everyone, but as the protocol side of defra is not up and running yet, this is not something we can actively code for really atm I think

@AndrewSisley AndrewSisley self-assigned this Sep 19, 2022
@jsimnz
Copy link
Member Author

jsimnz commented Sep 19, 2022

Would be nice if this can also be used to remove the P2P/broadcasting stuff from the db package too IMO

What do you mean? ATM, theres no P2P relation in the db package?

The "broadcaster" in the db package is a local one, that is a half-hazard attempt of a DB wide event system. So its likely that any format the solution to this issue of a event system takes, will still have a similar relationship in the DB package that the current "broadcaster" has.

The name "broadcaster" of the current setup is a bit of red-herring as it suggests actualy networking stuff, but its local events only

@AndrewSisley
Copy link
Contributor

I meant in the merkle package, I thought it was in db

@jsimnz
Copy link
Member Author

jsimnz commented Sep 19, 2022

It exists in both :)

Merkle:

func (base *baseMerkleCRDT) Broadcast(ctx context.Context, nd ipld.Node, delta core.Delta) error {

DB:
broadcaster corenet.Broadcaster

@AndrewSisley AndrewSisley changed the title System wide event queue to replace current broadcast. Events: System wide event queue to replace current broadcast. Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/db-system Related to the core system related components of the DB feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants