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

Move getReadVersion method to FDBRecordContext from FDBDatabase #698

Closed
alecgrieser opened this issue Aug 19, 2019 · 0 comments · Fixed by #699
Closed

Move getReadVersion method to FDBRecordContext from FDBDatabase #698

alecgrieser opened this issue Aug 19, 2019 · 0 comments · Fixed by #699
Assignees
Labels
enhancement New feature or request

Comments

@alecgrieser
Copy link
Contributor

At the moment, the method to get a read version lives in FDBDatabase (partially so that it can then use that to inform its read version tracking), but it probably makes more sense in FDBRecordContext. Then FDBRecordContext can call FDBDatabase's methods for tracking versions, which is already how commit version tracking is done. This also patches two possible misuses of the API that can happen: (1) if one calls FDBDatabase.getReadVersion with an FDBRecordContext from a different database, one can end up updating the wrong database's notion of what the most recent version is; and (2) if one calls getReadVersion with an FDBRecordContext that has been created with a cached version, one can end up accidentally updating the time value of the most recent time one has seen that version incorrectly.

@alecgrieser alecgrieser added the enhancement New feature or request label Aug 19, 2019
@alecgrieser alecgrieser self-assigned this Aug 19, 2019
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue Sep 6, 2019
…text from FDBDatabase

The old method is deprecated, and it has been replaced with a few different methods on FDBRecordContext. This is accompanied by additional information that tracks what the read version was in Java memory.
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue Sep 6, 2019
…ad versions

With FoundationDB#698 resolved so that read version tracking happens in `FDBRecordContext`, this was relatively easy. It just required making an explicit call to `FDBRecordContext::getReadVersion` from within the `OnlineIndexer`. I have verified that it shows up and has reasonable values in some of the logs from the `OnlineIndexer` tests.
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue Sep 6, 2019
…priority configurable

This adds a `setWeakReadSemantics` and a `setPriority` method to the `OnlineIndexer.Builder` method. Adding a way to set the weak read semantics was simple, as it was already a method on runners. To thread the priority through, I added a priority field to transactions and to runners, and then the online indexer exposes that. The `OnlineIndexer.Builder` method then sets the priority to `BATCH` (by default).

I was tempted to make the default weak read semantics one that set causal read risky to true. This would make sense for the online indexer, as it can actually tolerate stale reads (as everything is committed anyway), but it also benefits from seeing its own writes, which causal read risky actually guarantees. However, that is technically a behavior change, though it should be easy to add later if desired.

This is on top of FoundationDB#698 because (a) they conflict otherwise and (b) there are a few methods in FoundationDB#698 that this can use. It could use the underlying FDB methods, but I would prefer it use the new one instead. The extra instrumentation is also useful for determining if setting weak read semantics on the index build actually did anything as I could look at the GRV instrumentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant