Skip to content

Commit e682a02

Browse files
mfick-nvidiamfick-nvidia
committed
Pack.java: Recover more often in Pack.copyAsIs2()
The PACK class is designed to throw StoredObjectRepresentationNotAvailableException at times when it cannot find an object which previously was believed to be in its packfile and it is still possible for the caller, PackWriter.writeObjectImpl(), to retry copying the object from another file and potentially avoid causing a user facing error for this fairly common expected situation. This retry helps handle when repacking causes a packfile to be replaced by new files with the same objects. Improve copyAsIs2() to drastically make recovery possible in more situations. Once any data for a specific object, has been sent it is very difficult to recover from that object being relocated to another pack. But if a read error is detected in copyAsIs2() before sending the object header (and thus any data), then it should still be recoverable. Fix three places where we could have recovered because we hadn't sent the header yet, and adjust another place to send the header a bit later, after having read some data from the object successfully. Basically, if the header has not been written yet, throw StoredObjectRepresentationNotAvailableException to signal that this is still recoverable. These fixes should drastically improve the chances of recovery since due to unix file semantics, if the partial read succeeds, then the full read will very likely succeed. This is because while the file may no longer be open when the first read is done (the WindowCache may have evicted it), once the first read completes it will likely still be open and even if the file is deleted the WindowCache will continue to be able to read from it until it closes it. Change-Id: Ib87e294e0dbacf71b10db55be511e91963b4a84a Signed-off-by: Martin Fick <mfick@nvidia.com>
1 parent d34f8b5 commit e682a02

File tree

1 file changed

+153
-142
lines changed
  • org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file

1 file changed

+153
-142
lines changed

‎org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java

+153-142
Original file line numberDiff line numberDiff line change
@@ -416,185 +416,196 @@ private void copyAsIs2(PackOutputStream out, LocalObjectToPack src,
416416
final CRC32 crc2 = validate ? new CRC32() : null;
417417
final byte[] buf = out.getCopyBuffer();
418418

419+
boolean isHeaderWritten = false;
419420
// Rip apart the header so we can discover the size.
420421
//
421-
readFully(src.offset, buf, 0, 20, curs);
422-
int c = buf[0] & 0xff;
423-
final int typeCode = (c >> 4) & 7;
424-
long inflatedLength = c & 15;
425-
int shift = 4;
426-
int headerCnt = 1;
427-
while ((c & 0x80) != 0) {
428-
c = buf[headerCnt++] & 0xff;
429-
inflatedLength += ((long) (c & 0x7f)) << shift;
430-
shift += 7;
431-
}
432-
433-
if (typeCode == Constants.OBJ_OFS_DELTA) {
434-
do {
422+
try {
423+
readFully(src.offset, buf, 0, 20, curs);
424+
425+
int c = buf[0] & 0xff;
426+
final int typeCode = (c >> 4) & 7;
427+
long inflatedLength = c & 15;
428+
int shift = 4;
429+
int headerCnt = 1;
430+
while ((c & 0x80) != 0) {
435431
c = buf[headerCnt++] & 0xff;
436-
} while ((c & 128) != 0);
437-
if (validate) {
438-
assert(crc1 != null && crc2 != null);
439-
crc1.update(buf, 0, headerCnt);
440-
crc2.update(buf, 0, headerCnt);
432+
inflatedLength += ((long) (c & 0x7f)) << shift;
433+
shift += 7;
441434
}
442-
} else if (typeCode == Constants.OBJ_REF_DELTA) {
443-
if (validate) {
435+
436+
if (typeCode == Constants.OBJ_OFS_DELTA) {
437+
do {
438+
c = buf[headerCnt++] & 0xff;
439+
} while ((c & 128) != 0);
440+
if (validate) {
441+
assert(crc1 != null && crc2 != null);
442+
crc1.update(buf, 0, headerCnt);
443+
crc2.update(buf, 0, headerCnt);
444+
}
445+
} else if (typeCode == Constants.OBJ_REF_DELTA) {
446+
if (validate) {
447+
assert(crc1 != null && crc2 != null);
448+
crc1.update(buf, 0, headerCnt);
449+
crc2.update(buf, 0, headerCnt);
450+
}
451+
452+
readFully(src.offset + headerCnt, buf, 0, 20, curs);
453+
if (validate) {
454+
assert(crc1 != null && crc2 != null);
455+
crc1.update(buf, 0, 20);
456+
crc2.update(buf, 0, 20);
457+
}
458+
headerCnt += 20;
459+
} else if (validate) {
444460
assert(crc1 != null && crc2 != null);
445461
crc1.update(buf, 0, headerCnt);
446462
crc2.update(buf, 0, headerCnt);
447463
}
448464

449-
readFully(src.offset + headerCnt, buf, 0, 20, curs);
450-
if (validate) {
451-
assert(crc1 != null && crc2 != null);
452-
crc1.update(buf, 0, 20);
453-
crc2.update(buf, 0, 20);
454-
}
455-
headerCnt += 20;
456-
} else if (validate) {
457-
assert(crc1 != null && crc2 != null);
458-
crc1.update(buf, 0, headerCnt);
459-
crc2.update(buf, 0, headerCnt);
460-
}
465+
final long dataOffset = src.offset + headerCnt;
466+
final long dataLength = src.length;
467+
final long expectedCRC;
468+
final ByteArrayWindow quickCopy;
461469

462-
final long dataOffset = src.offset + headerCnt;
463-
final long dataLength = src.length;
464-
final long expectedCRC;
465-
final ByteArrayWindow quickCopy;
466-
467-
// Verify the object isn't corrupt before sending. If it is,
468-
// we report it missing instead.
469-
//
470-
try {
471-
quickCopy = curs.quickCopy(this, dataOffset, dataLength);
470+
// Verify the object isn't corrupt before sending. If it is,
471+
// we report it missing instead.
472+
//
473+
try {
474+
quickCopy = curs.quickCopy(this, dataOffset, dataLength);
472475

473-
if (validate && idx().hasCRC32Support()) {
474-
assert(crc1 != null);
475-
// Index has the CRC32 code cached, validate the object.
476-
//
477-
expectedCRC = idx().findCRC32(src);
478-
if (quickCopy != null) {
479-
quickCopy.crc32(crc1, dataOffset, (int) dataLength);
480-
} else {
481-
long pos = dataOffset;
482-
long cnt = dataLength;
483-
while (cnt > 0) {
484-
final int n = (int) Math.min(cnt, buf.length);
485-
readFully(pos, buf, 0, n, curs);
486-
crc1.update(buf, 0, n);
487-
pos += n;
488-
cnt -= n;
476+
if (validate && idx().hasCRC32Support()) {
477+
assert(crc1 != null);
478+
// Index has the CRC32 code cached, validate the object.
479+
//
480+
expectedCRC = idx().findCRC32(src);
481+
if (quickCopy != null) {
482+
quickCopy.crc32(crc1, dataOffset, (int) dataLength);
483+
} else {
484+
long pos = dataOffset;
485+
long cnt = dataLength;
486+
while (cnt > 0) {
487+
final int n = (int) Math.min(cnt, buf.length);
488+
readFully(pos, buf, 0, n, curs);
489+
crc1.update(buf, 0, n);
490+
pos += n;
491+
cnt -= n;
492+
}
489493
}
494+
if (crc1.getValue() != expectedCRC) {
495+
setCorrupt(src.offset);
496+
throw new CorruptObjectException(MessageFormat.format(
497+
JGitText.get().objectAtHasBadZlibStream,
498+
Long.valueOf(src.offset), getPackFile()));
499+
}
500+
} else if (validate) {
501+
// We don't have a CRC32 code in the index, so compute it
502+
// now while inflating the raw data to get zlib to tell us
503+
// whether or not the data is safe.
504+
//
505+
Inflater inf = curs.inflater();
506+
byte[] tmp = new byte[1024];
507+
if (quickCopy != null) {
508+
quickCopy.check(inf, tmp, dataOffset, (int) dataLength);
509+
} else {
510+
assert(crc1 != null);
511+
long pos = dataOffset;
512+
long cnt = dataLength;
513+
while (cnt > 0) {
514+
final int n = (int) Math.min(cnt, buf.length);
515+
readFully(pos, buf, 0, n, curs);
516+
crc1.update(buf, 0, n);
517+
inf.setInput(buf, 0, n);
518+
while (inf.inflate(tmp, 0, tmp.length) > 0)
519+
continue;
520+
pos += n;
521+
cnt -= n;
522+
}
523+
}
524+
if (!inf.finished() || inf.getBytesRead() != dataLength) {
525+
setCorrupt(src.offset);
526+
throw new EOFException(MessageFormat.format(
527+
JGitText.get().shortCompressedStreamAt,
528+
Long.valueOf(src.offset)));
529+
}
530+
assert(crc1 != null);
531+
expectedCRC = crc1.getValue();
532+
} else {
533+
expectedCRC = -1;
490534
}
491-
if (crc1.getValue() != expectedCRC) {
492-
setCorrupt(src.offset);
493-
throw new CorruptObjectException(MessageFormat.format(
494-
JGitText.get().objectAtHasBadZlibStream,
495-
Long.valueOf(src.offset), getPackFile()));
496-
}
497-
} else if (validate) {
498-
// We don't have a CRC32 code in the index, so compute it
499-
// now while inflating the raw data to get zlib to tell us
500-
// whether or not the data is safe.
535+
} catch (DataFormatException dataFormat) {
536+
setCorrupt(src.offset);
537+
538+
CorruptObjectException corruptObject = new CorruptObjectException(
539+
MessageFormat.format(
540+
JGitText.get().objectAtHasBadZlibStream,
541+
Long.valueOf(src.offset), getPackFile()),
542+
dataFormat);
543+
544+
throw new StoredObjectRepresentationNotAvailableException(
545+
corruptObject);
546+
}
547+
548+
if (quickCopy != null) {
549+
// The entire object fits into a single byte array window slice,
550+
// and we have it pinned. Write this out without copying.
501551
//
502-
Inflater inf = curs.inflater();
503-
byte[] tmp = new byte[1024];
504-
if (quickCopy != null) {
505-
quickCopy.check(inf, tmp, dataOffset, (int) dataLength);
506-
} else {
507-
assert(crc1 != null);
552+
out.writeHeader(src, inflatedLength);
553+
isHeaderWritten = true;
554+
quickCopy.write(out, dataOffset, (int) dataLength);
555+
556+
} else if (dataLength <= buf.length) {
557+
// Tiny optimization: Lots of objects are very small deltas or
558+
// deflated commits that are likely to fit in the copy buffer.
559+
//
560+
if (!validate) {
508561
long pos = dataOffset;
509562
long cnt = dataLength;
510563
while (cnt > 0) {
511564
final int n = (int) Math.min(cnt, buf.length);
512565
readFully(pos, buf, 0, n, curs);
513-
crc1.update(buf, 0, n);
514-
inf.setInput(buf, 0, n);
515-
while (inf.inflate(tmp, 0, tmp.length) > 0)
516-
continue;
517566
pos += n;
518567
cnt -= n;
519568
}
520569
}
521-
if (!inf.finished() || inf.getBytesRead() != dataLength) {
522-
setCorrupt(src.offset);
523-
throw new EOFException(MessageFormat.format(
524-
JGitText.get().shortCompressedStreamAt,
525-
Long.valueOf(src.offset)));
526-
}
527-
assert(crc1 != null);
528-
expectedCRC = crc1.getValue();
570+
out.writeHeader(src, inflatedLength);
571+
isHeaderWritten = true;
572+
out.write(buf, 0, (int) dataLength);
529573
} else {
530-
expectedCRC = -1;
531-
}
532-
} catch (DataFormatException dataFormat) {
533-
setCorrupt(src.offset);
534-
535-
CorruptObjectException corruptObject = new CorruptObjectException(
536-
MessageFormat.format(
537-
JGitText.get().objectAtHasBadZlibStream,
538-
Long.valueOf(src.offset), getPackFile()),
539-
dataFormat);
540-
541-
throw new StoredObjectRepresentationNotAvailableException(
542-
corruptObject);
543-
544-
} catch (IOException ioError) {
545-
throw new StoredObjectRepresentationNotAvailableException(ioError);
546-
}
547-
548-
if (quickCopy != null) {
549-
// The entire object fits into a single byte array window slice,
550-
// and we have it pinned. Write this out without copying.
551-
//
552-
out.writeHeader(src, inflatedLength);
553-
quickCopy.write(out, dataOffset, (int) dataLength);
554-
555-
} else if (dataLength <= buf.length) {
556-
// Tiny optimization: Lots of objects are very small deltas or
557-
// deflated commits that are likely to fit in the copy buffer.
558-
//
559-
if (!validate) {
574+
// Now we are committed to sending the object. As we spool it out,
575+
// check its CRC32 code to make sure there wasn't corruption between
576+
// the verification we did above, and us actually outputting it.
577+
//
560578
long pos = dataOffset;
561579
long cnt = dataLength;
562580
while (cnt > 0) {
563581
final int n = (int) Math.min(cnt, buf.length);
564582
readFully(pos, buf, 0, n, curs);
583+
if (validate) {
584+
assert(crc2 != null);
585+
crc2.update(buf, 0, n);
586+
}
587+
if (!isHeaderWritten) {
588+
out.writeHeader(src, inflatedLength);
589+
isHeaderWritten = true;
590+
}
591+
out.write(buf, 0, n);
565592
pos += n;
566593
cnt -= n;
567594
}
568-
}
569-
out.writeHeader(src, inflatedLength);
570-
out.write(buf, 0, (int) dataLength);
571-
} else {
572-
// Now we are committed to sending the object. As we spool it out,
573-
// check its CRC32 code to make sure there wasn't corruption between
574-
// the verification we did above, and us actually outputting it.
575-
//
576-
out.writeHeader(src, inflatedLength);
577-
long pos = dataOffset;
578-
long cnt = dataLength;
579-
while (cnt > 0) {
580-
final int n = (int) Math.min(cnt, buf.length);
581-
readFully(pos, buf, 0, n, curs);
582595
if (validate) {
583596
assert(crc2 != null);
584-
crc2.update(buf, 0, n);
597+
if (crc2.getValue() != expectedCRC) {
598+
throw new CorruptObjectException(MessageFormat.format(
599+
JGitText.get().objectAtHasBadZlibStream,
600+
Long.valueOf(src.offset), getPackFile()));
601+
}
585602
}
586-
out.write(buf, 0, n);
587-
pos += n;
588-
cnt -= n;
589603
}
590-
if (validate) {
591-
assert(crc2 != null);
592-
if (crc2.getValue() != expectedCRC) {
593-
throw new CorruptObjectException(MessageFormat.format(
594-
JGitText.get().objectAtHasBadZlibStream,
595-
Long.valueOf(src.offset), getPackFile()));
596-
}
604+
} catch (IOException ioError) {
605+
if (!isHeaderWritten) {
606+
throw new StoredObjectRepresentationNotAvailableException(ioError);
597607
}
608+
throw ioError;
598609
}
599610
}
600611

0 commit comments

Comments
 (0)