google / trillian

A transparent, highly scalable and cryptographically verifiable data store.
Apache License 2.0
3.53k stars 378 forks source link

Refactor logs operation #1640

Open pav-kv opened 5 years ago

pav-kv commented 5 years ago

LogOperation and its co-types are poorly designed:

The list of things we cache:

pav-kv commented 5 years ago

The current design has 3 types (OperationManager, SequencerManager and Sequencer), which all have their own caches for individual tree IDs.

I propose to shift "caching" one level up the stack, so that OperationManager merely does couple of things: tracks log IDs and the corresponding masterships, and for each active log ID has an Operation object. Operation object encapsulates all "cached" items that were previously scattered across different types (log names, signers, compact ranges, etc), and contains the code for handling only one log (effectively some blend of former SequencerManager and Sequencer, but with fixed Tree/treeID).

pav-kv commented 5 years ago

@Martin2112 @AlCutter WDYT?

Martin2112 commented 5 years ago

Yes the structure could be improved. If you're going to cache tree related things then it must be done in a way that's completely safe. You might want to make that a separate work item.

pav-kv commented 5 years ago

Yep. I will probably start from just restructuring the code with equivalent safety guarantees. For example, consider SequencerManager and its Signers cache: it is never cleared, and its items are never updated. Same with OperationManager and log names.

Once it's restructured, we could improve guarantees. E.g. if an Operation fails, we can delete it altogether, so on the next run it will be re-created with fresh signer, log name, etc.