fluree / server

Fluree Server - Operates Fluree in consensus, fault-tolerant, redundant
12 stars 1 forks source link

Refactor Consensus Protocol #48

Closed zonotope closed 5 months ago

zonotope commented 6 months ago

This patch among other things is a fix for the raft command queue being overwhelmed during a large reindexing process because of a lack of back pressure. The changes adding back pressure when forwarding new index file notifications to the consensus group are all contained in this commit

In the process of identifying changes we could make to the consensus api I found that, even though we had a TxGroup protocol for consensus, we had raft-specific implementation details throughout the code on either side of the protocol, so the rest of these changes are meant to isolate the raft implementation and tighten up the consensus protocol to make it more modular, extensible, and easier to add new consensus types in the future. I have refactored the TxGroup protocol to only contain the methods we need for general consensus, moved all raft-specific implementation details to the fluree.server.consensus.raft* namespace hierarchy, and removed unused code.

cap10morgan commented 5 months ago

@zonotope Looks like a potentially legit test failure? I think the expectation just needs to be updated (or the test itself to have something reasonable to expect now) due to the changes brought in by the db version bump.

zonotope commented 5 months ago

@zonotope Looks like a potentially legit test failure? I think the expectation just needs to be updated (or the test itself to have something reasonable to expect now) due to the changes brought in by the db version bump.

yep, a legit test failure brought on by the db version bump. I'm pretty sure it came from https://github.com/fluree/db/pull/744. I'm still thinking about the best way to fix it though, and the answer might be more philosophical than technical. if you query for an iri specifically, but we have no facts about it, should we give you nothing or an id map with that iri and nothing else? I could make the argument either way.

zonotope commented 5 months ago

@zonotope Looks like a potentially legit test failure? I think the expectation just needs to be updated (or the test itself to have something reasonable to expect now) due to the changes brought in by the db version bump.

yep, a legit test failure brought on by the db version bump. I'm pretty sure it came from fluree/db#744. I'm still thinking about the best way to fix it though, and the answer might be more philosophical than technical. if you query for an iri specifically, but we have no facts about it, should we give you nothing or an id map with that iri and nothing else? I could make the argument either way.

I went with the id map with nothing else instead of empty results. The subject exists by virtue of the user specifying the iri; we just don't know anything about it. The id map fits most closely with that I think.