Its unfortunately there until we get a chance to address it. I think the
comment explains it correctly:
// TODO: need to improve hack to be OK w/
// RateLimitingDirWrapper in between...
Actually I think Mark miller added some apis (getDelegate()) to wrapping
directories like NRTCachingDirectory and RateLimiting. Maybe this should
actually be pulled into a proper interface (DelegatingDirectory or
something), and MockDirectoryWrapper (which should also implement this i
suppose), could traverse the hierarchy and determine if any
NRTCachingDirectories exist. Then we wouldnt have to actually call fsync so
much in tests.
As far as syncing only some files, I think its fair for MDW to iterate the
collection of files and call sync on each one, with the possibility of a
random exception happening so only some are synced. It sorta does this
kinda thing with readBytes() etc already.
On Wed, May 8, 2013 at 3:56 PM, Shai Erera <ser...@gmail.com> wrote:
> So you say this true is there for a purpose? Is that what the TODO above
> refers to?
>
> If so, perhaps we should add some comment which better explains it.
>
> Also separately, currently sync either fails or succeeds to sync all
> files. Do you think perhaps it could randomly choose to throw IOE after
> syncing half the files?
>
> Shai
> On May 8, 2013 10:48 PM, "Robert Muir" <rcm...@gmail.com> wrote:
>
>> originally the if (true) did not exist.
>>
>> problem is (apart from system crash: where it matters to any directory),
>> NRTCachingDirectory's sync() actually causes stuff to go to disk. otherwise
>> it might just be sitting in RAM.
>>
>> however, then we added RateLimitingDirectory. so now you have possibility
>> of RateLimitingDirectory(NRTCaching(.... and other structures. So its not
>> easily possible to only sync sometimes without breaking things.
>>
>> On Wed, May 8, 2013 at 2:16 PM, Shai Erera <ser...@gmail.com> wrote:
>>
>>> Hi
>>>
>>> Look at this code from MDW.sync:
>>>
>>> if (true || LuceneTestCase.rarely(randomState) || delegate
>>> instanceof NRTCachingDirectory) {
>>> // don't wear out our hardware so much in tests.
>>> delegate.sync(names);
>>> }
>>>
>>> Is the 'if (true)' intentional or left there by mistake? The comment
>>> afterwards suggests it should not be there, but I wanted to confirm before
>>> I remove it.
>>>
>>> Shai
>>>
>>
>>
