> 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... 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 > >
