* JGit cache bug
@ 2009-04-18 17:05 Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 0/3 RFC] http://code.google.com/p/egit/issues/detail?id=79 Robin Rosenberg
0 siblings, 1 reply; 6+ messages in thread
From: Robin Rosenberg @ 2009-04-18 17:05 UTC (permalink / raw)
To: spearce, git
[-- Attachment #1: Type: text/plain, Size: 635 bytes --]
This is fun... ~
I cannot figure why things don't work with Patch #4? We are not
even multithreaded.
This is related to trying to fix http://code.google.com/p/egit/issues/detail?id=79 which is related to all
sorts of NPE problems that may occur in Eclipse when working with
EGit and C/msysgit in parallell.
Platform info:
# java version
java version "1.6.0_0"
IcedTea6 1.3.1 (6b12-0ubuntu6.4) Runtime Environment (build 1.6.0_0-b12)
OpenJDK Server VM (build 1.6.0_0-b12, mixed mode)
# uname -a
Linux sleipner 2.6.27-14-generic #1 SMP Wed Apr 15 18:59:16 UTC 2009 i686 GNU/Linux
I've tried to insert GC and sleep calls.
-- robin
[-- Attachment #2: 0001-Rescan-for-packs-once-when-failing-to-lookup-an-id.patch --]
[-- Type: text/x-patch, Size: 2987 bytes --]
From c557d6900d4952f5bee0d8dcab82a1730ba549eb Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Fri, 17 Apr 2009 09:34:39 +0200
Subject: [EGIT PATCH 1/4] Rescan for packs once when failing to lookup an id.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/lib/Repository.java | 31 ++++++++++++++-----
1 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index cfd92b8..8ff3b63 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -291,6 +291,21 @@ public ObjectLoader openObject(final AnyObjectId id)
*/
public ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)
throws IOException {
+ ObjectLoader loader = openObjectInPacks(curs, id);
+ if (loader != null)
+ return loader;
+ try {
+ return new UnpackedObjectLoader(this, id);
+ } catch (FileNotFoundException fnfe) {
+ System.out.println(new java.util.Date().toString() + ": failed to load " + id + ", rescanning for new packs");
+ if (scanForPacks())
+ return openObjectInPacks(curs, id);
+ return null;
+ }
+ }
+
+ private ObjectLoader openObjectInPacks(final WindowCursor curs, final AnyObjectId id)
+ throws IOException {
final PackFile[] packs = packs();
int k = packs.length;
while (k > 0) {
@@ -298,11 +313,7 @@ public ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)
if (ol != null)
return ol;
}
- try {
- return new UnpackedObjectLoader(this, id);
- } catch (FileNotFoundException fnfe) {
- return null;
- }
+ return null;
}
/**
@@ -813,10 +824,11 @@ synchronized (this) {
}
/**
- * Scan the object dirs, including alternates for packs
- * to use.
+ * Scan the object dirs, including alternates for packs to use.
+ *
+ * @return true if new packs were found
*/
- public void scanForPacks() {
+ public boolean scanForPacks() {
final ArrayList<PackFile> p = new ArrayList<PackFile>();
p.addAll(Arrays.asList(packs()));
for (final File d : objectsDirs())
@@ -824,9 +836,11 @@ public void scanForPacks() {
final PackFile[] arr = new PackFile[p.size()];
p.toArray(arr);
Arrays.sort(arr, PackFile.SORT);
+ boolean ret = packFileList.length != arr.length;
synchronized (this) {
packFileList = arr;
}
+ return ret;
}
private void scanForPacks(final File packDir, Collection<PackFile> packList) {
@@ -1200,5 +1214,6 @@ synchronized (allListeners) {
public void scanForRepoChanges() throws IOException {
getAllRefs(); // This will look for changes to refs
getIndex(); // This will detect changes in the index
+ scanForPacks(); // This will detect new packs
}
}
--
1.6.2.2.446.gfbdc0
[-- Attachment #3: 0002-Add-test-cases-dedicated-to-the-WindowCache.patch --]
[-- Type: text/x-patch, Size: 3389 bytes --]
From 92d37694970963ad6d0d5fbf4ec811630a58df90 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sat, 18 Apr 2009 16:15:28 +0200
Subject: [EGIT PATCH 2/4] Add test cases dedicated to the WindowCache
---
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 72 ++++++++++++++++++++
1 files changed, 72 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
new file mode 100644
index 0000000..a438d46
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
@@ -0,0 +1,72 @@
+package org.spearce.jgit.lib;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.revwalk.RevObject;
+import org.spearce.jgit.revwalk.RevWalk;
+
+public class WindowCacheTest extends RepositoryTestCase {
+
+ public void setUp() throws Exception {
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ windowCacheConfig.setPackedGitOpenFiles(1);
+ WindowCache.reconfigure(windowCacheConfig);
+ super.setUp();
+ }
+
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ WindowCache.reconfigure(windowCacheConfig);
+ }
+
+ /*
+ * Add pack, find objects in new pack Replace packs, find objects in another
+ * pack than original, don't choke on lost pack
+ */
+ public void testObjectInNewPack() throws IncorrectObjectTypeException,
+ IOException {
+ Repository extra = createNewEmptyRepo();
+ RevObject o1 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData");
+ PackWriter packWriter = new PackWriter(extra, new TextProgressMonitor());
+ packWriter.addObject(o1);
+ ObjectId name = packWriter.computeName();
+ File packFileName = fullPackFileName(db, name, ".pack");
+ FileOutputStream pos = new FileOutputStream(packFileName);
+ packWriter.writePack(pos);
+ pos.close();
+ File idxfname = fullPackFileName(db, name, ".idx");
+ FileOutputStream ios = new FileOutputStream(idxfname);
+ packWriter.writeIndex(ios);
+ ios.close();
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+ }
+
+ private File fullPackFileName(Repository repository, ObjectId hash,
+ String suffix) {
+ return new File(new File(repository.getObjectsDirectory(), "pack"),
+ "pack-" + hash.name() + suffix);
+ }
+
+ private RevObject writeBlob(final Repository repo, final Repository notIn,
+ final String data) throws IOException {
+ RevWalk revWalk = new RevWalk(repo);
+ byte[] bytes = data.getBytes(Constants.CHARACTER_ENCODING);
+ ObjectId id = new ObjectWriter(repo).writeBlob(bytes.length,
+ new ByteArrayInputStream(bytes));
+ try {
+ assertNull(
+ "Oops, We want new objects that we do not have yet, for this test!",
+ new RevWalk(notIn).parseAny(id));
+ } catch (MissingObjectException e) {
+ // Ok
+ }
+ return revWalk.lookupBlob(id);
+ }
+}
--
1.6.2.2.446.gfbdc0
[-- Attachment #4: 0003-Fails-like-this-in-the-debugger-expected-but-runs-ok.patch --]
[-- Type: text/x-patch, Size: 5894 bytes --]
From 81f9174ebf605030ea95d18bb81dc1ece98bf290 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sat, 18 Apr 2009 18:29:23 +0200
Subject: [EGIT PATCH 3/4] Fails like this in the debugger (expected), but runs ok outside the debugger.
Breakpoints in the following places. Wait a second or two everytime it stops
WindowCacheTest [line: 101] - testObjectMovedToNewPack()
WindowedFile [line: 283] - cacheOpen()
WindowedFile [line: 301] - cacheClose()
java.io.FileNotFoundException: /home/me/SW/EGIT/org.spearce.jgit.test/trash/trash1240071609317.0/.git/objects/pack/pack-fc4dadd6673a2740f43468f868ba5dd39fdeba69.pack (Filen eller katalogen finns inte)
at java.io.RandomAccessFile.open(Native Method)
at java.io.RandomAccessFile.<init>(RandomAccessFile.java:212)
at org.spearce.jgit.lib.WindowedFile.cacheOpen(WindowedFile.java:283)
at org.spearce.jgit.lib.WindowCache.getImpl(WindowCache.java:249)
at org.spearce.jgit.lib.WindowCache.get(WindowCache.java:222)
at org.spearce.jgit.lib.WindowCursor.pin(WindowCursor.java:148)
at org.spearce.jgit.lib.WindowCursor.copy(WindowCursor.java:82)
at org.spearce.jgit.lib.WindowedFile.read(WindowedFile.java:176)
at org.spearce.jgit.lib.WindowedFile.readFully(WindowedFile.java:203)
at org.spearce.jgit.lib.PackFile.reader(PackFile.java:311)
at org.spearce.jgit.lib.PackFile.get(PackFile.java:147)
at org.spearce.jgit.lib.Repository.openObjectInPacks(Repository.java:312)
at org.spearce.jgit.lib.Repository.openObject(Repository.java:294)
at org.spearce.jgit.revwalk.RevWalk.parseAny(RevWalk.java:702)
at org.spearce.jgit.lib.WindowCacheTest.testObjectMovedToNewPack(WindowCacheTest.java:115)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at junit.framework.TestCase.runTest(TestCase.java:164)
at junit.framework.TestCase.runBare(TestCase.java:130)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:120)
at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
---
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 53 ++++++++++++++++++++
1 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
index a438d46..0cd55c9 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
@@ -48,6 +48,59 @@ public void testObjectInNewPack() throws IncorrectObjectTypeException,
assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
}
+ /*
+ * Add pack, find objects in new pack Replace packs, find objects in another
+ * pack than original, don't choke on lost pack
+ */
+ public void testObjectMovedToNewPack() throws IncorrectObjectTypeException,
+ IOException {
+ Repository extra = createNewEmptyRepo();
+ RevObject o1 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData1");
+ PackWriter packWriter1 = new PackWriter(extra,
+ new TextProgressMonitor());
+ packWriter1.addObject(o1);
+ ObjectId name1 = packWriter1.computeName();
+ File packFileName1 = fullPackFileName(db, name1, ".pack");
+ FileOutputStream pos1 = new FileOutputStream(packFileName1);
+ packWriter1.writePack(pos1);
+ pos1.close();
+ File idxfname1 = fullPackFileName(db, name1, ".idx");
+ FileOutputStream ios1 = new FileOutputStream(idxfname1);
+ packWriter1.writeIndex(ios1);
+ ios1.close();
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+
+ // Ok, lets repack this repo
+ // create an additional object
+ RevObject o2 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData2");
+ PackWriter packWriter2 = new PackWriter(extra,
+ new TextProgressMonitor());
+ packWriter2.addObject(o1);
+ packWriter2.addObject(o2);
+ ObjectId name2 = packWriter2.computeName();
+ File packFileName2 = fullPackFileName(db, name2, ".pack");
+ FileOutputStream pos2 = new FileOutputStream(packFileName2);
+ packWriter2.writePack(pos2);
+ pos2.close();
+ File idxfname2 = fullPackFileName(db, name2, ".idx");
+ FileOutputStream ios2 = new FileOutputStream(idxfname2);
+ packWriter2.writeIndex(ios2);
+ ios2.close();
+ assertEquals(o2.name(), new RevWalk(db).parseAny(o2).name());
+
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ windowCacheConfig.setPackedGitOpenFiles(1);
+ WindowCache.reconfigure(windowCacheConfig);
+ packFileName1.delete();
+ idxfname1.delete();
+
+ // Now here is the interesting thing. Will git figure the new
+ // object exists in the new pack, and not the old one.
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+ }
+
private File fullPackFileName(Repository repository, ObjectId hash,
String suffix) {
return new File(new File(repository.getObjectsDirectory(), "pack"),
--
1.6.2.2.446.gfbdc0
[-- Attachment #5: 0004-Simulate-breakpoints.patch --]
[-- Type: text/x-patch, Size: 1865 bytes --]
From 44638f9060d845a208638ac75d392a168b87c0df Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sat, 18 Apr 2009 19:00:32 +0200
Subject: [EGIT PATCH 4/4] Simulate breakpoints
---
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 7 +++++++
.../src/org/spearce/jgit/lib/WindowedFile.java | 7 +++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
index 0cd55c9..eba1aed 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
@@ -98,6 +98,13 @@ public void testObjectMovedToNewPack() throws IncorrectObjectTypeException,
// Now here is the interesting thing. Will git figure the new
// object exists in the new pack, and not the old one.
+ try {
+ Thread.sleep(2000);
+ } catch (InterruptedException e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ System.gc();
assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index 938f44c..f7377d1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -280,6 +280,13 @@ public void close() {
}
void cacheOpen() throws IOException {
+ try {
+ Thread.sleep(2000);
+ } catch (InterruptedException e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ System.gc();
fd = new RandomAccessFile(fPath, "r");
length = fd.length();
try {
--
1.6.2.2.446.gfbdc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 0/3 RFC] http://code.google.com/p/egit/issues/detail?id=79
2009-04-18 17:05 JGit cache bug Robin Rosenberg
@ 2009-04-19 18:30 ` Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 1/3] Add test cases dedicated to the WindowCache Robin Rosenberg
0 siblings, 1 reply; 6+ messages in thread
From: Robin Rosenberg @ 2009-04-19 18:30 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
Here is a possible solution, though I have a problem with the test case. I'd
like to invent something better. The problem with the "Simulate breakpoints"
patch is that it patches core functions and not just the test case. Using AOP
would solve it, but that is too much for a single test case.
Robin Rosenberg (3):
Add test cases dedicated to the WindowCache
Simulate breakpoints
Rescan for packs and retry once if object lookup scan fails
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 132 ++++++++++++++++++++
.../src/org/spearce/jgit/lib/Repository.java | 76 +++++++++--
.../src/org/spearce/jgit/lib/WindowedFile.java | 7 +
3 files changed, 202 insertions(+), 13 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
^ permalink raw reply [flat|nested] 6+ messages in thread
* [JGIT PATCH 1/3] Add test cases dedicated to the WindowCache
2009-04-19 18:30 ` [JGIT PATCH 0/3 RFC] http://code.google.com/p/egit/issues/detail?id=79 Robin Rosenberg
@ 2009-04-19 18:30 ` Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 2/3] Simulate breakpoints Robin Rosenberg
0 siblings, 1 reply; 6+ messages in thread
From: Robin Rosenberg @ 2009-04-19 18:30 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
Fails like this in the debugger (expected), but runs ok outside the debugger.
Breakpoints in the following places. Wait a second or two everytime it stops
WindowCacheTest [line: 101] - testObjectMovedToNewPack()
WindowedFile [line: 283] - cacheOpen()
java.io.FileNotFoundException: /home/me/SW/EGIT/org.spearce.jgit.test/trash/trash1240071609317.0/.git/objects/pack/pack-fc4dadd6673a2740f43468f868ba5dd39fdeba69.pack (Filen eller katalogen finns inte)
at java.io.RandomAccessFile.open(Native Method)
at java.io.RandomAccessFile.<init>(RandomAccessFile.java:212)
at org.spearce.jgit.lib.WindowedFile.cacheOpen(WindowedFile.java:283)
at org.spearce.jgit.lib.WindowCache.getImpl(WindowCache.java:249)
at org.spearce.jgit.lib.WindowCache.get(WindowCache.java:222)
at org.spearce.jgit.lib.WindowCursor.pin(WindowCursor.java:148)
at org.spearce.jgit.lib.WindowCursor.copy(WindowCursor.java:82)
at org.spearce.jgit.lib.WindowedFile.read(WindowedFile.java:176)
at org.spearce.jgit.lib.WindowedFile.readFully(WindowedFile.java:203)
at org.spearce.jgit.lib.PackFile.reader(PackFile.java:311)
at org.spearce.jgit.lib.PackFile.get(PackFile.java:147)
at org.spearce.jgit.lib.Repository.openObjectInPacks(Repository.java:312)
at org.spearce.jgit.lib.Repository.openObject(Repository.java:294)
at org.spearce.jgit.revwalk.RevWalk.parseAny(RevWalk.java:702)
at org.spearce.jgit.lib.WindowCacheTest.testObjectMovedToNewPack(WindowCacheTest.java:115)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at junit.framework.TestCase.runTest(TestCase.java:164)
at junit.framework.TestCase.runBare(TestCase.java:130)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:120)
at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
---
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 125 ++++++++++++++++++++
1 files changed, 125 insertions(+), 0 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
new file mode 100644
index 0000000..0cd55c9
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
@@ -0,0 +1,125 @@
+package org.spearce.jgit.lib;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import org.spearce.jgit.errors.IncorrectObjectTypeException;
+import org.spearce.jgit.errors.MissingObjectException;
+import org.spearce.jgit.revwalk.RevObject;
+import org.spearce.jgit.revwalk.RevWalk;
+
+public class WindowCacheTest extends RepositoryTestCase {
+
+ public void setUp() throws Exception {
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ windowCacheConfig.setPackedGitOpenFiles(1);
+ WindowCache.reconfigure(windowCacheConfig);
+ super.setUp();
+ }
+
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ WindowCache.reconfigure(windowCacheConfig);
+ }
+
+ /*
+ * Add pack, find objects in new pack Replace packs, find objects in another
+ * pack than original, don't choke on lost pack
+ */
+ public void testObjectInNewPack() throws IncorrectObjectTypeException,
+ IOException {
+ Repository extra = createNewEmptyRepo();
+ RevObject o1 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData");
+ PackWriter packWriter = new PackWriter(extra, new TextProgressMonitor());
+ packWriter.addObject(o1);
+ ObjectId name = packWriter.computeName();
+ File packFileName = fullPackFileName(db, name, ".pack");
+ FileOutputStream pos = new FileOutputStream(packFileName);
+ packWriter.writePack(pos);
+ pos.close();
+ File idxfname = fullPackFileName(db, name, ".idx");
+ FileOutputStream ios = new FileOutputStream(idxfname);
+ packWriter.writeIndex(ios);
+ ios.close();
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+ }
+
+ /*
+ * Add pack, find objects in new pack Replace packs, find objects in another
+ * pack than original, don't choke on lost pack
+ */
+ public void testObjectMovedToNewPack() throws IncorrectObjectTypeException,
+ IOException {
+ Repository extra = createNewEmptyRepo();
+ RevObject o1 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData1");
+ PackWriter packWriter1 = new PackWriter(extra,
+ new TextProgressMonitor());
+ packWriter1.addObject(o1);
+ ObjectId name1 = packWriter1.computeName();
+ File packFileName1 = fullPackFileName(db, name1, ".pack");
+ FileOutputStream pos1 = new FileOutputStream(packFileName1);
+ packWriter1.writePack(pos1);
+ pos1.close();
+ File idxfname1 = fullPackFileName(db, name1, ".idx");
+ FileOutputStream ios1 = new FileOutputStream(idxfname1);
+ packWriter1.writeIndex(ios1);
+ ios1.close();
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+
+ // Ok, lets repack this repo
+ // create an additional object
+ RevObject o2 = writeBlob(extra, db,
+ "nihuioijoisuoiudyosyudoiuyusyuoiyewData2");
+ PackWriter packWriter2 = new PackWriter(extra,
+ new TextProgressMonitor());
+ packWriter2.addObject(o1);
+ packWriter2.addObject(o2);
+ ObjectId name2 = packWriter2.computeName();
+ File packFileName2 = fullPackFileName(db, name2, ".pack");
+ FileOutputStream pos2 = new FileOutputStream(packFileName2);
+ packWriter2.writePack(pos2);
+ pos2.close();
+ File idxfname2 = fullPackFileName(db, name2, ".idx");
+ FileOutputStream ios2 = new FileOutputStream(idxfname2);
+ packWriter2.writeIndex(ios2);
+ ios2.close();
+ assertEquals(o2.name(), new RevWalk(db).parseAny(o2).name());
+
+ WindowCacheConfig windowCacheConfig = new WindowCacheConfig();
+ windowCacheConfig.setPackedGitOpenFiles(1);
+ WindowCache.reconfigure(windowCacheConfig);
+ packFileName1.delete();
+ idxfname1.delete();
+
+ // Now here is the interesting thing. Will git figure the new
+ // object exists in the new pack, and not the old one.
+ assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
+ }
+
+ private File fullPackFileName(Repository repository, ObjectId hash,
+ String suffix) {
+ return new File(new File(repository.getObjectsDirectory(), "pack"),
+ "pack-" + hash.name() + suffix);
+ }
+
+ private RevObject writeBlob(final Repository repo, final Repository notIn,
+ final String data) throws IOException {
+ RevWalk revWalk = new RevWalk(repo);
+ byte[] bytes = data.getBytes(Constants.CHARACTER_ENCODING);
+ ObjectId id = new ObjectWriter(repo).writeBlob(bytes.length,
+ new ByteArrayInputStream(bytes));
+ try {
+ assertNull(
+ "Oops, We want new objects that we do not have yet, for this test!",
+ new RevWalk(notIn).parseAny(id));
+ } catch (MissingObjectException e) {
+ // Ok
+ }
+ return revWalk.lookupBlob(id);
+ }
+}
--
1.6.2.2.446.gfbdc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 2/3] Simulate breakpoints
2009-04-19 18:30 ` [JGIT PATCH 1/3] Add test cases dedicated to the WindowCache Robin Rosenberg
@ 2009-04-19 18:30 ` Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 3/3] Rescan for packs and retry once if object lookup scan fails Robin Rosenberg
0 siblings, 1 reply; 6+ messages in thread
From: Robin Rosenberg @ 2009-04-19 18:30 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
---
.../tst/org/spearce/jgit/lib/WindowCacheTest.java | 7 +++++++
.../src/org/spearce/jgit/lib/WindowedFile.java | 7 +++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
index 0cd55c9..eba1aed 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/WindowCacheTest.java
@@ -98,6 +98,13 @@ public void testObjectMovedToNewPack() throws IncorrectObjectTypeException,
// Now here is the interesting thing. Will git figure the new
// object exists in the new pack, and not the old one.
+ try {
+ Thread.sleep(2000);
+ } catch (InterruptedException e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ System.gc();
assertEquals(o1.name(), new RevWalk(db).parseAny(o1).name());
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index 938f44c..f7377d1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -280,6 +280,13 @@ public void close() {
}
void cacheOpen() throws IOException {
+ try {
+ Thread.sleep(2000);
+ } catch (InterruptedException e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ System.gc();
fd = new RandomAccessFile(fPath, "r");
length = fd.length();
try {
--
1.6.2.2.446.gfbdc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 3/3] Rescan for packs and retry once if object lookup scan fails
2009-04-19 18:30 ` [JGIT PATCH 2/3] Simulate breakpoints Robin Rosenberg
@ 2009-04-19 18:30 ` Robin Rosenberg
2009-04-20 14:32 ` Shawn O. Pearce
0 siblings, 1 reply; 6+ messages in thread
From: Robin Rosenberg @ 2009-04-19 18:30 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
We do this by catching the FileNotFoundException thrown by the lower
layers an rescan for packs.
A lookup failure may happen if no pack contains the requested object, so
the loose object lookup fails. The other case is that one of the packs
we use has been deleted by a repack. In either case there is a good chance
that there is a new pack or loose object that contains the object.
Therefore we scan for new packs, invalidating an old ones that may have
been deleted and retry the lookup.
At the same time we stop scanning for new packs regularly since we now
have another mechanism for finding them.
---
.../src/org/spearce/jgit/lib/Repository.java | 76 ++++++++++++++++----
1 files changed, 63 insertions(+), 13 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index cfd92b8..ec4226c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -100,6 +100,8 @@
private GitIndex index;
private List<RepositoryListener> listeners = new Vector<RepositoryListener>(); // thread safe
+
+ private int packFileListGeneration;
static private List<RepositoryListener> allListeners = new Vector<RepositoryListener>(); // thread safe
/**
@@ -291,6 +293,40 @@ public ObjectLoader openObject(final AnyObjectId id)
*/
public ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)
throws IOException {
+ boolean scanned = false;
+ try {
+ ObjectLoader loader = openObjectInPacks(curs, id);
+ if (loader != null)
+ return loader;
+ } catch (FileNotFoundException fnfe) {
+ System.out.println(new java.util.Date().toString() + ": failed to load " + id + ", rescanning for new packs");
+ scanned = true;
+ if (scanForPacks()) {
+ ObjectLoader loader = openObjectInPacks(curs, id);
+ if (loader != null)
+ return loader;
+ }
+ }
+ try {
+ return new UnpackedObjectLoader(this, id);
+ } catch (FileNotFoundException fnfe) {
+ System.out.println(new java.util.Date().toString() + ": failed to load " + id + ", rescanning for new packs");
+ if (!scanned && scanForPacks()) {
+ ObjectLoader loader = openObjectInPacks(curs, id);
+ if (loader != null)
+ return loader;
+ try {
+ return new UnpackedObjectLoader(this, id);
+ } catch (FileNotFoundException fnfe2) {
+ return null;
+ }
+ }
+ return null;
+ }
+ }
+
+ private ObjectLoader openObjectInPacks(final WindowCursor curs, final AnyObjectId id)
+ throws IOException {
final PackFile[] packs = packs();
int k = packs.length;
while (k > 0) {
@@ -298,11 +334,7 @@ public ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)
if (ol != null)
return ol;
}
- try {
- return new UnpackedObjectLoader(this, id);
- } catch (FileNotFoundException fnfe) {
- return null;
- }
+ return null;
}
/**
@@ -813,23 +845,39 @@ synchronized (this) {
}
/**
- * Scan the object dirs, including alternates for packs
- * to use.
+ * Scan the object dirs, including alternates for packs to use.
+ *
+ * @return true if new packs were found
*/
- public void scanForPacks() {
+ public boolean scanForPacks() {
+ int oldPackFileListGeneration = packFileListGeneration;
final ArrayList<PackFile> p = new ArrayList<PackFile>();
- p.addAll(Arrays.asList(packs()));
for (final File d : objectsDirs())
- scanForPacks(new File(d, "pack"), p);
+ scanForPacks(new File(d, "pack"), p, packFileList);
final PackFile[] arr = new PackFile[p.size()];
p.toArray(arr);
Arrays.sort(arr, PackFile.SORT);
synchronized (this) {
+ if (!samePackfileArray(packFileList, arr))
+ packFileListGeneration++;
packFileList = arr;
}
+ return oldPackFileListGeneration != packFileListGeneration;
}
- private void scanForPacks(final File packDir, Collection<PackFile> packList) {
+ private static<T> boolean samePackfileArray(PackFile[] oldList, PackFile[] newList) {
+ if (oldList == null)
+ return false;
+ if (oldList.length != newList.length)
+ return false;
+ for (int i = 0; i < oldList.length; ++i) {
+ if (!oldList[i].getPackFile().equals(newList[i].getPackFile()))
+ return false;
+ }
+ return true;
+ }
+
+ private void scanForPacks(final File packDir, Collection<PackFile> packList, PackFile[] oldPackFileList) {
final String[] idxList = packDir.list(new FilenameFilter() {
public boolean accept(final File baseDir, final String n) {
// Must match "pack-[0-9a-f]{40}.idx" to be an index.
@@ -851,9 +899,11 @@ public boolean accept(final File baseDir, final String n) {
continue;
}
- for (final PackFile p : packList) {
- if (packFile.equals(p.getPackFile()))
+ for (final PackFile p : oldPackFileList) {
+ if (packFile.equals(p.getPackFile())) {
+ packList.add(p);
continue SCAN;
+ }
}
packList.add(new PackFile(idxFile, packFile));
--
1.6.2.2.446.gfbdc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [JGIT PATCH 3/3] Rescan for packs and retry once if object lookup scan fails
2009-04-19 18:30 ` [JGIT PATCH 3/3] Rescan for packs and retry once if object lookup scan fails Robin Rosenberg
@ 2009-04-20 14:32 ` Shawn O. Pearce
0 siblings, 0 replies; 6+ messages in thread
From: Shawn O. Pearce @ 2009-04-20 14:32 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> .../src/org/spearce/jgit/lib/Repository.java | 76 ++++++++++++++++----
> 1 files changed, 63 insertions(+), 13 deletions(-)
NAK.
I see a number of things I don't like about this implementation
as it stands. I'll try to work up a counter patch today.
At a glance, the new logic for openObject is pretty twisted
to follow. It mostly makes sense, but I had hoped for something
cleaner to read.
I'm really worried about the case where a pack file stays with
the same name, but its contents and index have been recreated,
and thus our existing PackFile object is invalid for access to it.
Your implementation as written would keep the same PackFile in
memory, and we'd lose access to those objects.
You also pointed out some concerns about windows stuck in the
WindowCache. I'm equally worried about that.
--
Shawn.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-04-20 14:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-18 17:05 JGit cache bug Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 0/3 RFC] http://code.google.com/p/egit/issues/detail?id=79 Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 1/3] Add test cases dedicated to the WindowCache Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 2/3] Simulate breakpoints Robin Rosenberg
2009-04-19 18:30 ` [JGIT PATCH 3/3] Rescan for packs and retry once if object lookup scan fails Robin Rosenberg
2009-04-20 14:32 ` Shawn O. Pearce
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).