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

Add model for interaction between Lucene & Version Map on replicas #28

Closed
wants to merge 24 commits into from

Conversation

DaveCTurner
Copy link
Contributor

This adds a model for how we should track sequence numbers for indexing and deletions in the replica, using the Version Map and Lucene. It includes an optimisation that can avoid the need to perform these (relatively expensive) checks for append-only operations when it is safe to do so.

The replica does not inspect the version field, so it can simply be removed.
@DaveCTurner
Copy link
Contributor Author

Summary of verbal review with @bleskes:

The document versions are not relevant for the replica - they are a relic of older models that modelled more of the primary too. Removed in 0165d83.

However, the model of Lucene needs refinement: we model replacement of a document as if calling the update API, but there is also an add API which doesn't delete any previous versions first. The add API can only be used if we're sure there is nothing that needs removing, but is more efficient so should be used where possible. The add API can be modelled as having the same effect as the update API but with an additional assertion that there is no document to overwrite.

@DaveCTurner
Copy link
Contributor Author

@dnhatn it's likely you will be involved in the implementation of this so it might be useful for you to contemplate this PR too. Not sure what level you're at with this kind of modelling but I have time to walk you through it if needed.

@dnhatn
Copy link
Member

dnhatn commented Feb 18, 2018

@DaveCTurner Thanks for pinging me. I think I will focus on the implementation. I will surely have a look on this but mainly for learning purpose.

Copy link
Contributor Author

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ywelsch @bleskes @dnhatn This is ready for another look.


else

AdvanceLocalCheckPoint:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the advancing of the local checkpoint to be a separate process?


Termination == <>(\A self \in ProcSet: pc[self] = "Done")

\* END TRANSLATION
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want some more invariants below? At the moment they're mostly about properties at termination, which is simple and ought to be sufficient, but we could have more if desired.

@DaveCTurner
Copy link
Contributor Author

Closing this in favour of #29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants