Re: Evists persisted deltas from memory (Incomplete).

1 view
Skip to first unread message

soren...@gmail.com

unread,
Sep 9, 2011, 11:17:55 AM9/9/11
to veg...@gmail.com, wave-protocol...@googlegroups.com

http://codereview.waveprotocol.org/615001/diff/1/2
File
src/org/waveprotocol/box/server/waveserver/DeltaStoreBasedWaveletState.java
(right):

http://codereview.waveprotocol.org/615001/diff/1/2#newcode345
Line 345: ImmutableList<WaveletDeltaRecord> deltasTopersist =
deltas.build();
deltasTopersist -> deltasToPersist

http://codereview.waveprotocol.org/615001/diff/1/2#newcode352
Line 352: HashedVersion persitedAtVersion =
persistedDelta.getAppliedAtVersion();
persitedAtVersion -> persistedAtVersion

http://codereview.waveprotocol.org/615001/diff/1002/1003#newcode303
Line 303: "Applied version %s doesn't match current version %s",
appliedAtVersion, currentVersion);
It's sad if we always have to go to disk. Can we first check if we
happen to have it in memory?

http://codereview.waveprotocol.org/615001/diff/1002/1003#newcode386
Line 386:
I wonder if there is any risk that anyone else tries to access these
maps while we delete the deltas here.

http://codereview.waveprotocol.org/615001/diff/1002/1003#newcode389
Line 389:
this can't happen, according to the way deltasToPersist is constructed,
right?

to make sure, you can replace this test with

Preconditions.checkState(!d.getAppliedAtVersion().equals(version));

in line 377

http://codereview.waveprotocol.org/615001/diff/1002/1004
File src/org/waveprotocol/box/server/waveserver/WaveletDeltaRecord.java
(right):

http://codereview.waveprotocol.org/615001/diff/1002/1004#newcode49
Line 49: Preconditions.checkNotNull(applied, "null appliedAtVersion");
good catch

http://codereview.waveprotocol.org/615001/diff/1002/1005
File src/org/waveprotocol/box/server/waveserver/WaveletState.java
(right):

http://codereview.waveprotocol.org/615001/diff/1002/1005#newcode92
Line 92: throws PersistenceException;
I don't think we should make getTransformedDeltaHistory,
getAppliedDelta, and getAppliedDeltaHistory blocking but rather by
asynchronous, by returning a ListenableFuture or taking a listener
callback argument. But it will require a lot of code changes of the
callers, so that will be a lot of work.

Maybe I'm wrong, but I worry that we can't just let all the callers
block, unless we audit that none of them are holding locks or stuff that
will get things into a jam.

http://codereview.waveprotocol.org/615001

Reply all
Reply to author
Forward
0 new messages