[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] preparing 2.0

On Fri, Jun 1, 2012 at 7:36 AM, Markus Duft <markus.duft@xxxxxxxxxx> wrote:
> On 05/26/2012 09:17 AM, Robin Rosenberg wrote:
> [snip]
>>
>> Eclipse rarely dies on out-of-memory, even when it's not handled. Sometimes you just get an stack trace on stderr.
>>
>> An IOException could also lead to missing objects and perhaps we should log it.
>>
>>                 } catch (IOException e) {
>>                     // Assume the pack is corrupted.
>>                     //
>>                     removePack(p);
>>
>
> i added some logs here and there, and i'll see. what i found out by searching (for getChannel()), is that there is still some channel java.nio involved here and there with pack file reading, which is (as we all now) very vulnerable to ClosedByInterruptExceptions. I don't yet know whether this is the problem, but i have a bad feeling about that. Also, Shawn, from your last blog post on your homepage i see you had a really bad encounter with the problem already back in 2010, /and/ it seems that it is even quite a related problem.

:-)

That post was about JGit, and I removed it from the normal pack file
code path. We now use a RandomAccessFile to read from a pack, and we
use a lock around that file, so only 1 thread can read from a pack
file at any given time. We try to avoid that lock by using a large
cache in big JGit servers. If the necessary data is already loaded in
the cache, the thread doesn't need to take a lock.

> maybe we should eleminate all the nio stuff completely, just to make sure :) i just don't know how, otherwise i would do.

If mmap is enabled we still use NIO I think. Robin (at least in the
past) has said he likes the mmap feature. I avoid it at all costs.

There are a few places in the write code path for loose objects where
we still use NIO. But these should be safe against
ClosedByInterruptExceptions in the sense that only one thread is
accessing that file, and if the file throws any sort of IOException
its closed and deleted, because it hasn't yet been committed to the
repository.