Author: vgritsenko Date: Fri Mar 23 15:58:29 2007 New Revision: 521932 URL: http://svn.apache.org/viewvc?view=rev&rev=521932 Log: fix rarely happening error in paged
Modified: xml/xindice/trunk/java/src/org/apache/xindice/core/filer/HashFiler.java xml/xindice/trunk/java/src/org/apache/xindice/core/filer/Paged.java xml/xindice/trunk/java/src/org/apache/xindice/server/XindiceServlet.java xml/xindice/trunk/java/tests/src/org/apache/xindice/core/filer/FilerTestBase.java Modified: xml/xindice/trunk/java/src/org/apache/xindice/core/filer/HashFiler.java URL: http://svn.apache.org/viewvc/xml/xindice/trunk/java/src/org/apache/xindice/core/filer/HashFiler.java?view=diff&rev=521932&r1=521931&r2=521932 ============================================================================== --- xml/xindice/trunk/java/src/org/apache/xindice/core/filer/HashFiler.java (original) +++ xml/xindice/trunk/java/src/org/apache/xindice/core/filer/HashFiler.java Fri Mar 23 15:58:29 2007 @@ -165,6 +165,7 @@ break; } + // Check the chain long pageNum = ph.getNextCollision(); if (pageNum == NO_PAGE) { // Reached end of chain, add new page @@ -179,7 +180,7 @@ break; } - // Go to next page in chain + // Go to the next page in chain p = getPage(pageNum); } @@ -192,10 +193,6 @@ } ph.setModified(t); ph.setStatus(RECORD); - - // Write modifications to the page header before exiting synchronization block - // This will prevent other threads from getting this same page - p.write(); } return p; @@ -217,7 +214,7 @@ writeValue(p, value); p = null; } catch (Exception e) { - throw new FilerException(FaultCodes.DBE_CANNOT_CREATE, "Exception: " + e); + throw new FilerException(FaultCodes.DBE_CANNOT_CREATE, "Exception: " + e, e); } finally { // FIXME It's not enough. At this point, new record could have been added to the chain if (p != null) { @@ -269,7 +266,7 @@ long pageNum = hash % fileHeader.getPageCount(); Page page = getPage(pageNum); - synchronized(page) { + synchronized (page) { HashPageHeader prevHead = null; HashPageHeader pageHead; Modified: xml/xindice/trunk/java/src/org/apache/xindice/core/filer/Paged.java URL: http://svn.apache.org/viewvc/xml/xindice/trunk/java/src/org/apache/xindice/core/filer/Paged.java?view=diff&rev=521932&r1=521931&r2=521932 ============================================================================== --- xml/xindice/trunk/java/src/org/apache/xindice/core/filer/Paged.java (original) +++ xml/xindice/trunk/java/src/org/apache/xindice/core/filer/Paged.java Fri Mar 23 15:58:29 2007 @@ -162,16 +162,22 @@ */ private Configuration config; - // TODO: This is not a cache right now, but a way to assure that only one page instance at most exists in memory at all times. /** - * Cache of recently read pages. + * Map of pages in use. Guarantees that page with same number will be loaded + * into memory just once, allowing to synchronize on page objects to guarantee + * no two threads are writing into same page at once. * - * Cache contains weak references to the Page objects, keys are page numbers (Long objects). - * Access synchronized by this map itself. + * <p>Map contains weak references to the Page objects, keys are pages themselves. + * Access is synchronized by the [EMAIL PROTECTED] #pagesLock}. */ private final Map pages = new WeakHashMap(); /** + * Lock for synchronizing access to the [EMAIL PROTECTED] #pages} map. + */ + private final Object pagesLock = new Object(); + + /** * Cache of modified pages waiting to be written out. * Access is synchronized by the [EMAIL PROTECTED] #dirtyLock}. */ @@ -332,19 +338,22 @@ * @throws IOException if an Exception occurs */ protected final Page getPage(long pageNum) throws IOException { - final Long lp = new Long(pageNum); + final Page lp = new Page(new Long(pageNum)); Page p = null; - synchronized (pages) { + synchronized (pagesLock) { // Check if page is already loaded in the page cache WeakReference ref = (WeakReference) pages.get(lp); if (ref != null) { p = (Page) ref.get(); + // Fall through to p.read(). Even if page present in the pages + // map, it still has to be read - it could be that it was just + // added to the map but read() was not called yet. } - // If not found, create it and add it to the page cache + // If not found, create it and add it to the pages cache if (p == null) { - p = new Page(lp); - pages.put(p.pageNum, new WeakReference(p)); + p = new Page(new Long(pageNum)); + pages.put(p, new WeakReference(p)); } } @@ -1249,7 +1258,7 @@ private int dataPos; - public Page(Long pageNum) { + private Page(Long pageNum) { this.header = createPageHeader(); this.pageNum = pageNum; this.offset = fileHeader.headerSize + (pageNum.longValue() * fileHeader.pageSize); @@ -1366,6 +1375,33 @@ // No synchronization: pageNum is final. public int compareTo(Object o) { return (int) (pageNum.longValue() - ((Page) o).pageNum.longValue()); + } + + /** + * Return page hash code, which is hash code of its [EMAIL PROTECTED] #pageNum}. + * + * @return Page hash code + */ + public int hashCode() { + return pageNum.hashCode(); + } + + /** + * Pages are equal if they are the same or have equal pageNum. + * + * @param obj Another page + * @return true if pages are equal + */ + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + + if (obj instanceof Page) { + return pageNum.longValue() == ((Page) obj).pageNum.longValue(); + } + + return false; } } } Modified: xml/xindice/trunk/java/src/org/apache/xindice/server/XindiceServlet.java URL: http://svn.apache.org/viewvc/xml/xindice/trunk/java/src/org/apache/xindice/server/XindiceServlet.java?view=diff&rev=521932&r1=521931&r2=521932 ============================================================================== --- xml/xindice/trunk/java/src/org/apache/xindice/server/XindiceServlet.java (original) +++ xml/xindice/trunk/java/src/org/apache/xindice/server/XindiceServlet.java Fri Mar 23 15:58:29 2007 @@ -229,7 +229,7 @@ String path = servletConfig.getInitParameter(Xindice.PROP_XINDICE_CONFIGURATION); if (path != null) { - InputStream inputStream = null; + InputStream inputStream; if (path.startsWith("/")) { // Absolute file path log.debug("Loading configuration from filesystem path " + path); @@ -244,10 +244,7 @@ try { configurationDocument = DOMParser.toDocument(inputStream); } finally { - try { - inputStream.close(); - } catch (IOException ignored) { - } + inputStream.close(); } } else { log.debug("Loading the standard configuration"); Modified: xml/xindice/trunk/java/tests/src/org/apache/xindice/core/filer/FilerTestBase.java URL: http://svn.apache.org/viewvc/xml/xindice/trunk/java/tests/src/org/apache/xindice/core/filer/FilerTestBase.java?view=diff&rev=521932&r1=521931&r2=521932 ============================================================================== --- xml/xindice/trunk/java/tests/src/org/apache/xindice/core/filer/FilerTestBase.java (original) +++ xml/xindice/trunk/java/tests/src/org/apache/xindice/core/filer/FilerTestBase.java Fri Mar 23 15:58:29 2007 @@ -88,10 +88,11 @@ if (filer != null) { filer.close(); } + filer = null; String[] files = ROOT.list(); for (int i = 0; i < files.length; i++) { - new File(ROOT, files[i]).delete(); + assertTrue(new File(ROOT, files[i]).delete()); } ROOT.delete(); super.tearDown(); @@ -286,8 +287,9 @@ } public void testConcurrentInsert() throws Exception { - assertTrue(filer.getRecordCount() == 0); + assertEquals(0, filer.getRecordCount()); + final Object go = new Object(); final int THREADS = 30; final int ITERATIONS = 65; @@ -296,6 +298,12 @@ final int threadID = i; threads[i] = new Thread() { public void run() { + synchronized (go) { + try { + go.wait(); + } catch (InterruptedException e) { /* ignored */ } + } + for (int ii = 0; ii < ITERATIONS; ii++) { Key key = new Key("T" + threadID + "I" + ii); Value value = new Value("<test thread=\"" + threadID + "\" iteration=\"" + ii + "\"/>"); @@ -305,26 +313,24 @@ e.printStackTrace(); } } - // System.out.println(getName() + " done."); + // System.out.println(this.getName() + " done."); } }; threads[i].setName("FilerTest" + i); + threads[i].start(); } // Start all the threads at once - for (int i = 0; i < THREADS; i++) { - threads[i].start(); + Thread.sleep(250); + synchronized (go) { + go.notifyAll(); } - Thread.sleep(1000); - for (int i = 0; i < THREADS; i++) { threads[i].join(); } - filer.flush(); - // Check results - assertEquals(filer.getRecordCount(), THREADS * ITERATIONS); + assertEquals(THREADS * ITERATIONS, filer.getRecordCount()); for (int i = 0; i < THREADS; i++) { for (int ii = 0; ii < ITERATIONS; ii++) { Key key = new Key("T" + i + "I" + ii);