On Thu, Jul 18, 2013 at 8:40 AM, Ali Lown <a...@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 >> >> >
