incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Gentle <jose...@gmail.com>
Subject Re: Review Request 12678: Remove all Memory-backed Persistence Stores
Date Thu, 18 Jul 2013 19:47:33 GMT
On Thu, Jul 18, 2013 at 8:40 AM, Ali Lown <ali@lown.me.uk> wrote:
>> On July 18, 2013, 3:13 p.m., Vicente J. Ruiz Jurado wrote:
>> > From my point of view the File Persistence still have bugs that we have not
addressed and aren't present in Memory Store, so I usually compare the implementations to
try to see the differences and fix it.
>> >
>> > In general there are more corruptions in waves and more indexing problems.
>> >
>> > So to remove this code before fix these issues scare me.
>
> I agree that anecdotally I see corruptions and indexing problems using the file persistence
systems.
> But anecdotally, they don't disappear completely using the memory systems either...

Sounds like we need more tests. Although if the problems don't
disappear completely using the other file systems its possible they're
concurrency issues.

-J

> It is impossible to be running an actual server using the memory persistence systems,
so it serves little (no) purpose remaining in the codebase.
>
> [If it is simply that you use the code for reference, it will always be there in the
source control system, just not taking up space (and time) during compilation+release].
>
>
> - Ali
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12678/#review23383
> -----------------------------------------------------------
>
>
> On July 17, 2013, 3:20 p.m., Ali Lown wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/12678/
>> -----------------------------------------------------------
>>
>> (Updated July 17, 2013, 3:20 p.m.)
>>
>>
>> Review request for wave, Bruno Gonzalez, Vicente J. Ruiz Jurado, and Yuri Zelikov.
>>
>>
>> Repository: wave-git
>>
>>
>> Description
>> -------
>>
>> Following on from the MongoDB removal, I then pondered why these Memory-backed stores
still exist.
>> The only reason I could come up with was the test-suite, so this patch also fixes
the test-suite to use File-backed stores.
>>
>> This removes a second implementation of everything (which we have to 'test' before
every release) making that easier, removes hundreds of LOC to maintain and adjust if we want
to add a new field/etc.
>>
>> I had also started to notice discrepancies in behaviour between implementation backings
(e.g. exceptional case behaviour differences for lucene/memory search backing).
>>
>> Nobody is running an actual server using memory-backing, so we no longer have a need
to keep this code. (It was primarily developed during bootstrapping until some actual persistence
exists).
>>
>> I also considered removing the config switches etc, but decided to not delete too
much in one review :P
>>
>>
>> Diffs
>> -----
>>
>>   server.config.example 19ba8b2
>>   src/org/waveprotocol/box/server/CoreSettings.java 5beaf65
>>   src/org/waveprotocol/box/server/SearchModule.java 2de0ef9
>>   src/org/waveprotocol/box/server/persistence/FakePermissiveAccountStore.java fcefc18
>>   src/org/waveprotocol/box/server/persistence/PersistenceModule.java a430570
>>   src/org/waveprotocol/box/server/persistence/memory/MemoryDeltaCollection.java da6a85e
>>   src/org/waveprotocol/box/server/persistence/memory/MemoryDeltaStore.java db1606a
>>   src/org/waveprotocol/box/server/persistence/memory/MemoryStore.java de0cbda
>>   src/org/waveprotocol/box/server/waveserver/ImportServlet.java 67c4ee4
>>   src/org/waveprotocol/box/server/waveserver/MemoryPerUserWaveViewHandlerImpl.java
d995d5e
>>   src/org/waveprotocol/box/server/waveserver/MemoryWaveIndexerImpl.java c5c6fe4
>>   test/org/waveprotocol/box/server/authentication/AccountStoreLoginModuleTest.java
48f905f
>>   test/org/waveprotocol/box/server/authentication/SessionManagerTest.java 461cd03
>>   test/org/waveprotocol/box/server/persistence/memory/AccountStoreTest.java f89f08a
>>   test/org/waveprotocol/box/server/persistence/memory/CertPathStoreTest.java 19fabbd
>>   test/org/waveprotocol/box/server/persistence/memory/DeltaStoreTest.java 2e85646
>>   test/org/waveprotocol/box/server/rpc/AuthenticationServletTest.java 2e39d2d
>>   test/org/waveprotocol/box/server/rpc/FetchServletTest.java 2ae4dbc
>>   test/org/waveprotocol/box/server/rpc/UserRegistrationServletTest.java bd83db8
>>   test/org/waveprotocol/box/server/waveserver/CertificateManagerImplTest.java 75ac795
>>   test/org/waveprotocol/box/server/waveserver/DeltaStoreBasedWaveletStateTest.java
6b09778
>>   test/org/waveprotocol/box/server/waveserver/LocalWaveletContainerImplTest.java
5355e2b
>>   test/org/waveprotocol/box/server/waveserver/MemoryPerUserWaveViewProviderTest.java
89006a4
>>   test/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImplTest.java 30e0c2d
>>   test/org/waveprotocol/box/server/waveserver/WaveMapTest.java e161490
>>   test/org/waveprotocol/box/server/waveserver/WaveServerTest.java 1da4f7b
>>   test/org/waveprotocol/box/server/waveserver/WaveletContainerTest.java 92d5baa
>>   test/org/waveprotocol/box/server/waveserver/WaveletStateTestBase.java db3e86e
>>
>> Diff: https://reviews.apache.org/r/12678/diff/
>>
>>
>> Testing
>> -------
>>
>> Builds and passes test suite (Junit maintains temporary directories for us).
>> The composition of all 7 of these 'related' (but independent) patches is verified
to still work as a wave server.
>>
>>
>> Thanks,
>>
>> Ali Lown
>>
>>
>

Mime
View raw message