Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1416110) +++ lucene/CHANGES.txt (working copy) @@ -59,6 +59,12 @@ invalidated on reopen if even a single delete was posted against the segment). (Robert Muir) +* LUCENE-4575: Replace IndexWriter's commit/prepareCommit versions that take + commitData with setCommitData(). That allows committing changes to IndexWriter + even if the commitData is the only thing that changes. It also means that you + should set the commitData before any call to commit() or prepareCommit() if you + want it to be committed. (Shai Erera) + New Features * LUCENE-4226: New experimental StoredFieldsFormat that compresses chunks of Index: lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/CommitIndexTask.java =================================================================== --- lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/CommitIndexTask.java (revision 1416110) +++ lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/CommitIndexTask.java (working copy) @@ -49,7 +49,8 @@ public int doLogic() throws Exception { IndexWriter iw = getRunData().getIndexWriter(); if (iw != null) { - iw.commit(commitUserData); + iw.setCommitData(commitUserData); + iw.commit(); } return 1; Index: lucene/core/src/java/org/apache/lucene/index/IndexCommit.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/IndexCommit.java (revision 1416110) +++ lucene/core/src/java/org/apache/lucene/index/IndexCommit.java (working copy) @@ -107,7 +107,7 @@ public abstract long getGeneration(); /** Returns userData, previously passed to {@link - * IndexWriter#commit(Map)} for this commit. Map is + * IndexWriter#setCommitData(Map)} for this commit. Map is * String -> String. */ public abstract Map getUserData() throws IOException; Index: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (revision 1416110) +++ lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (working copy) @@ -954,7 +954,7 @@ } if (doFlush) { - commitInternal(null); + commitInternal(); } if (infoStream.isEnabled("IW")) { @@ -2553,18 +2553,6 @@ */ protected void doBeforeFlush() throws IOException {} - /** Expert: prepare for commit. - * - *

NOTE: if this method hits an OutOfMemoryError - * you should immediately close the writer. See above for details.

- * - * @see #prepareCommit(Map) */ - public final void prepareCommit() throws IOException { - ensureOpen(); - prepareCommit(null); - } - /**

Expert: prepare for commit, specifying * commitUserData Map (String -> String). This does the * first phase of 2-phase commit. This method does all @@ -2576,29 +2564,22 @@ * #rollback()} to revert the commit and undo all changes * done since the writer was opened.

* - *

You can also just call {@link #commit(Map)} directly + *

You can also just call {@link #commit()} directly * without prepareCommit first in which case that method * will internally call prepareCommit. * *

NOTE: if this method hits an OutOfMemoryError * you should immediately close the writer. See above for details.

- * - * @param commitUserData Opaque Map (String->String) - * that's recorded into the segments file in the index, - * and retrievable by {@link - * IndexCommit#getUserData}. Note that when - * IndexWriter commits itself during {@link #close}, the - * commitUserData is unchanged (just carried over from - * the prior commit). If this is null then the previous - * commitUserData is kept. Also, the commitUserData will - * only "stick" if there are actually changes in the - * index to commit. */ - public final void prepareCommit(Map commitUserData) throws IOException { - ensureOpen(false); + public final void prepareCommit() throws IOException { + ensureOpen(); + prepareCommitInternal(); + } + private void prepareCommitInternal() throws IOException { synchronized(commitLock) { + ensureOpen(false); if (infoStream.isEnabled("IW")) { infoStream.message("IW", "prepareCommit: flush"); infoStream.message("IW", " index before flush " + segString()); @@ -2688,10 +2669,23 @@ } } - startCommit(toCommit, commitUserData); + startCommit(toCommit); } } - + + /** + * Sets the commit user data map. That method is considered a transaction by + * {@link IndexWriter} and will be {@link #commit() committed} even if no other + * changes were made to the writer instance. + *

+ * NOTE: the map is cloned internally, therefore altering the map's + * contents after calling this method has no effect. + */ + public final void setCommitData(Map commitUserData) { + segmentInfos.setUserData(new HashMap(commitUserData)); + ++changeCount; + } + // Used only by commit and prepareCommit, below; lock // order is commitLock -> IW private final Object commitLock = new Object(); @@ -2724,29 +2718,13 @@ * href="#OOME">above for details.

* * @see #prepareCommit - * @see #commit(Map) */ public final void commit() throws IOException { - commit(null); - } - - /** Commits all changes to the index, specifying a - * commitUserData Map (String -> String). This just - * calls {@link #prepareCommit(Map)} (if you didn't - * already call it) and then {@link #commit}. - * - *

NOTE: if this method hits an OutOfMemoryError - * you should immediately close the writer. See above for details.

- */ - public final void commit(Map commitUserData) throws IOException { - ensureOpen(); - - commitInternal(commitUserData); + commitInternal(); } - private final void commitInternal(Map commitUserData) throws IOException { + private final void commitInternal() throws IOException { if (infoStream.isEnabled("IW")) { infoStream.message("IW", "commit: start"); @@ -2763,7 +2741,7 @@ if (infoStream.isEnabled("IW")) { infoStream.message("IW", "commit: now prepare"); } - prepareCommit(commitUserData); + prepareCommitInternal(); } else { if (infoStream.isEnabled("IW")) { infoStream.message("IW", "commit: already prepared"); @@ -3906,7 +3884,7 @@ * if it wasn't already. If that succeeds, then we * prepare a new segments_N file but do not fully commit * it. */ - private void startCommit(final SegmentInfos toSync, final Map commitUserData) throws IOException { + private void startCommit(final SegmentInfos toSync) throws IOException { assert testPoint("startStartCommit"); assert pendingCommit == null; @@ -3939,10 +3917,6 @@ } assert filesExist(toSync); - - if (commitUserData != null) { - toSync.setUserData(commitUserData); - } } assert testPoint("midStartCommit"); Index: lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java (revision 1416110) +++ lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java (working copy) @@ -103,8 +103,8 @@ *
  • SegCodec is the {@link Codec#getName() name} of the Codec that encoded * this segment.
    • *
  • CommitUserData stores an optional user-supplied opaque - * Map<String,String> that was passed to {@link IndexWriter#commit(java.util.Map)} - * or {@link IndexWriter#prepareCommit(java.util.Map)}.
    • + * Map<String,String> that was passed to + * {@link IndexWriter#setCommitData(java.util.Map)}. * *

    * @@ -903,7 +903,7 @@ /** Return {@code userData} saved with this commit. * - * @see IndexWriter#commit(Map) + * @see IndexWriter#commit() */ public Map getUserData() { return userData; Index: lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommit.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommit.java (revision 1416110) +++ lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommit.java (working copy) @@ -1,7 +1,6 @@ package org.apache.lucene.index; import java.io.IOException; -import java.util.Map; /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -38,18 +37,6 @@ public void prepareCommit() throws IOException; /** - * Like {@link #commit()}, but takes an additional commit data to be included - * w/ the commit. - *

    - * NOTE: some implementations may not support any custom data to be - * included w/ the commit and may discard it altogether. Consult the actual - * implementation documentation for verifying if this is supported. - * - * @see #prepareCommit() - */ - public void prepareCommit(Map commitData) throws IOException; - - /** * The second phase of a 2-phase commit. Implementations should ideally do * very little work in this method (following {@link #prepareCommit()}, and * after it returns, the caller can assume that the changes were successfully @@ -58,15 +45,6 @@ public void commit() throws IOException; /** - * Like {@link #commit()}, but takes an additional commit data to be included - * w/ the commit. - * - * @see #commit() - * @see #prepareCommit(Map) - */ - public void commit(Map commitData) throws IOException; - - /** * Discards any changes that have occurred since the last commit. In a 2-phase * commit algorithm, where one of the objects failed to {@link #commit()} or * {@link #prepareCommit()}, this method is used to roll all other objects Index: lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommitTool.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommitTool.java (revision 1416110) +++ lucene/core/src/java/org/apache/lucene/index/TwoPhaseCommitTool.java (working copy) @@ -1,7 +1,6 @@ package org.apache.lucene.index; import java.io.IOException; -import java.util.Map; /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -32,44 +31,6 @@ private TwoPhaseCommitTool() {} /** - * A wrapper of a {@link TwoPhaseCommit}, which delegates all calls to the - * wrapped object, passing the specified commitData. This object is useful for - * use with {@link TwoPhaseCommitTool#execute(TwoPhaseCommit...)} if one would - * like to store commitData as part of the commit. - */ - public static final class TwoPhaseCommitWrapper implements TwoPhaseCommit { - - private final TwoPhaseCommit tpc; - private final Map commitData; - - /** Sole constructor. */ - public TwoPhaseCommitWrapper(TwoPhaseCommit tpc, Map commitData) { - this.tpc = tpc; - this.commitData = commitData; - } - - public void prepareCommit() throws IOException { - prepareCommit(commitData); - } - - public void prepareCommit(Map commitData) throws IOException { - tpc.prepareCommit(this.commitData); - } - - public void commit() throws IOException { - commit(commitData); - } - - public void commit(Map commitData) throws IOException { - tpc.commit(this.commitData); - } - - public void rollback() throws IOException { - tpc.rollback(); - } - } - - /** * Thrown by {@link TwoPhaseCommitTool#execute(TwoPhaseCommit...)} when an * object fails to prepareCommit(). */ Index: lucene/core/src/test/org/apache/lucene/index/TestDeletionPolicy.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestDeletionPolicy.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestDeletionPolicy.java (working copy) @@ -212,7 +212,8 @@ IndexWriter writer = new IndexWriter(dir, conf); Map commitData = new HashMap(); commitData.put("commitTime", String.valueOf(System.currentTimeMillis())); - writer.commit(commitData); + writer.setCommitData(commitData); + writer.commit(); writer.close(); long lastDeleteTime = 0; @@ -234,7 +235,8 @@ } commitData = new HashMap(); commitData.put("commitTime", String.valueOf(System.currentTimeMillis())); - writer.commit(commitData); + writer.setCommitData(commitData); + writer.commit(); writer.close(); Thread.sleep((int) (1000.0*(SECONDS/5.0))); Index: lucene/core/src/test/org/apache/lucene/index/TestDirectoryReaderReopen.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestDirectoryReaderReopen.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestDirectoryReaderReopen.java (working copy) @@ -554,13 +554,15 @@ writer.addDocument(doc); Map data = new HashMap(); data.put("index", i+""); - writer.commit(data); + writer.setCommitData(data); + writer.commit(); } for(int i=0;i<4;i++) { writer.deleteDocuments(new Term("id", ""+i)); Map data = new HashMap(); data.put("index", (4+i)+""); - writer.commit(data); + writer.setCommitData(data); + writer.commit(); } writer.close(); Index: lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java (working copy) @@ -23,6 +23,7 @@ import java.io.StringReader; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -1932,4 +1933,25 @@ w.close(); dir.close(); } + + // LUCENE-4575 + public void testCommitWithUserDataOnly() throws Exception { + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, null)); + writer.commit(); // first commit to complete IW create transaction. + + // this should store the commit data, even though no other changes were made + writer.setCommitData(new HashMap() {{ + put("key", "value"); + }}); + writer.commit(); + + DirectoryReader r = DirectoryReader.open(dir); + assertEquals("value", r.getIndexCommit().getUserData().get("key")); + + writer.close(); + r.close(); + dir.close(); + } + } Index: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterCommit.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestIndexWriterCommit.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestIndexWriterCommit.java (working copy) @@ -428,12 +428,13 @@ // commit to "first" Map commitData = new HashMap(); commitData.put("tag", "first"); - w.commit(commitData); + w.setCommitData(commitData); + w.commit(); // commit to "second" w.addDocument(doc); commitData.put("tag", "second"); - w.commit(commitData); + w.setCommitData(commitData); w.close(); // open "first" with IndexWriter @@ -454,7 +455,7 @@ // commit IndexWriter to "third" w.addDocument(doc); commitData.put("tag", "third"); - w.commit(commitData); + w.setCommitData(commitData); w.close(); // make sure "second" commit is still there @@ -635,7 +636,7 @@ TestIndexWriter.addDoc(w); Map data = new HashMap(); data.put("label", "test1"); - w.commit(data); + w.setCommitData(data); w.close(); r = DirectoryReader.open(dir); Index: lucene/core/src/test/org/apache/lucene/index/TestTransactionRollback.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestTransactionRollback.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestTransactionRollback.java (working copy) @@ -30,7 +30,6 @@ import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.store.Directory; -import org.apache.lucene.store.MockDirectoryWrapper; import org.apache.lucene.util.Bits; import org.apache.lucene.util.LuceneTestCase; @@ -72,7 +71,7 @@ new RollbackDeletionPolicy(id)).setIndexCommit(last)); Map data = new HashMap(); data.put("index", "Rolled back to 1-"+id); - w.commit(data); + w.setCommitData(data); w.close(); } @@ -142,7 +141,8 @@ if (currentRecordId%10 == 0) { Map data = new HashMap(); data.put("index", "records 1-"+currentRecordId); - w.commit(data); + w.setCommitData(data); + w.commit(); } } Index: lucene/core/src/test/org/apache/lucene/index/TestTwoPhaseCommitTool.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestTwoPhaseCommitTool.java (revision 1416110) +++ lucene/core/src/test/org/apache/lucene/index/TestTwoPhaseCommitTool.java (working copy) @@ -18,10 +18,8 @@ */ import java.io.IOException; -import java.util.HashMap; import java.util.Map; -import org.apache.lucene.index.TwoPhaseCommitTool.TwoPhaseCommitWrapper; import org.apache.lucene.util.LuceneTestCase; public class TestTwoPhaseCommitTool extends LuceneTestCase { @@ -117,27 +115,6 @@ } } - public void testWrapper() throws Exception { - // tests that TwoPhaseCommitWrapper delegates prepare/commit w/ commitData - TwoPhaseCommitImpl impl = new TwoPhaseCommitImpl(false, false, false); - HashMap commitData = new HashMap(); - TwoPhaseCommitWrapper wrapper = new TwoPhaseCommitWrapper(impl, commitData); - - wrapper.prepareCommit(); - assertSame(commitData, impl.prepareCommitData); - - // wrapper should ignore passed commitData - wrapper.prepareCommit(new HashMap()); - assertSame(commitData, impl.prepareCommitData); - - wrapper.commit(); - assertSame(commitData, impl.commitData); - - // wrapper should ignore passed commitData - wrapper.commit(new HashMap()); - assertSame(commitData, impl.commitData); - } - public void testNullTPCs() throws Exception { int numObjects = random().nextInt(4) + 3; // between [3, 6] TwoPhaseCommit[] tpcs = new TwoPhaseCommit[numObjects]; Index: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java (revision 1416110) +++ lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java (working copy) @@ -176,7 +176,7 @@ /** * Retrieve user committed data. * - * @see TaxonomyWriter#commit(Map) + * @see TaxonomyWriter#setCommitData(Map) */ public abstract Map getCommitUserData() throws IOException; Index: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyWriter.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyWriter.java (revision 1416110) +++ lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyWriter.java (working copy) @@ -2,6 +2,7 @@ import java.io.Closeable; import java.io.IOException; +import java.util.Map; import org.apache.lucene.index.TwoPhaseCommit; @@ -104,4 +105,14 @@ */ public int getSize(); + /** + * Sets the commit user data map. That method is considered a transaction and + * will be {@link #commit() committed} even if no other changes were made to + * the writer instance. + *

    + * NOTE: the map is cloned internally, therefore altering the map's + * contents after calling this method has no effect. + */ + public void setCommitData(Map commitUserData); + } Index: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java (revision 1416110) +++ lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java (working copy) @@ -129,6 +129,8 @@ private volatile ParentArray parentArray; private volatile int nextID; + private Map commitData; + /** Reads the commit data from a Directory. */ private static Map readCommitData(Directory dir) throws IOException { SegmentInfos infos = new SegmentInfos(); @@ -353,7 +355,8 @@ @Override public synchronized void close() throws IOException { if (!isClosed) { - indexWriter.commit(combinedCommitData(null)); + indexWriter.setCommitData(combinedCommitData()); + indexWriter.commit(); doClose(); } } @@ -660,39 +663,28 @@ } } - /** - * Calling commit() ensures that all the categories written so far are - * visible to a reader that is opened (or reopened) after that call. - * When the index is closed(), commit() is also implicitly done. - * See {@link TaxonomyWriter#commit()} - */ @Override public synchronized void commit() throws IOException { ensureOpen(); - indexWriter.commit(combinedCommitData(null)); + indexWriter.setCommitData(combinedCommitData()); + indexWriter.commit(); } /** * Combine original user data with that of the taxonomy creation time */ - private Map combinedCommitData(Map userData) { + private Map combinedCommitData() { Map m = new HashMap(); - if (userData != null) { - m.putAll(userData); + if (commitData != null) { + m.putAll(commitData); } m.put(INDEX_EPOCH, Long.toString(indexEpoch)); return m; } - /** - * Like commit(), but also store properties with the index. These properties - * are retrievable by {@link DirectoryTaxonomyReader#getCommitUserData}. - * See {@link TaxonomyWriter#commit(Map)}. - */ @Override - public synchronized void commit(Map commitUserData) throws IOException { - ensureOpen(); - indexWriter.commit(combinedCommitData(commitUserData)); + public void setCommitData(Map commitUserData) { + this.commitData = new HashMap(commitUserData); } /** @@ -702,18 +694,9 @@ @Override public synchronized void prepareCommit() throws IOException { ensureOpen(); - indexWriter.prepareCommit(combinedCommitData(null)); + indexWriter.setCommitData(combinedCommitData()); + indexWriter.prepareCommit(); } - - /** - * Like above, and also prepares to store user data with the index. - * See {@link IndexWriter#prepareCommit(Map)} - */ - @Override - public synchronized void prepareCommit(Map commitUserData) throws IOException { - ensureOpen(); - indexWriter.prepareCommit(combinedCommitData(commitUserData)); - } @Override public int getSize() { Index: lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java =================================================================== --- lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java (revision 1416110) +++ lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyWriter.java (working copy) @@ -89,7 +89,7 @@ taxoWriter.addCategory(new CategoryPath("b")); Map userCommitData = new HashMap(); userCommitData.put("testing", "1 2 3"); - taxoWriter.commit(userCommitData); + taxoWriter.setCommitData(userCommitData); taxoWriter.close(); DirectoryReader r = DirectoryReader.open(dir); assertEquals("2 categories plus root should have been committed to the underlying directory", 3, r.numDocs()); @@ -104,7 +104,7 @@ // that the taxonomy index has been recreated. taxoWriter = new DirectoryTaxonomyWriter(dir, OpenMode.CREATE_OR_APPEND, NO_OP_CACHE); taxoWriter.addCategory(new CategoryPath("c")); // add a category so that commit will happen - taxoWriter.commit(new HashMap(){{ + taxoWriter.setCommitData(new HashMap(){{ put("just", "data"); }}); taxoWriter.close(); @@ -163,9 +163,10 @@ private void touchTaxo(DirectoryTaxonomyWriter taxoWriter, CategoryPath cp) throws IOException { taxoWriter.addCategory(cp); - taxoWriter.commit(new HashMap(){{ + taxoWriter.setCommitData(new HashMap(){{ put("just", "data"); }}); + taxoWriter.commit(); } @Test Index: solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java =================================================================== --- solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (revision 1416110) +++ solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (working copy) @@ -449,8 +449,8 @@ final Map commitData = new HashMap(); commitData.put(SolrIndexWriter.COMMIT_TIME_MSEC_KEY, String.valueOf(System.currentTimeMillis())); - - iw.get().prepareCommit(commitData); + iw.get().setCommitData(commitData); + iw.get().prepareCommit(); } finally { iw.decref(); } @@ -525,7 +525,8 @@ final Map commitData = new HashMap(); commitData.put(SolrIndexWriter.COMMIT_TIME_MSEC_KEY, String.valueOf(System.currentTimeMillis())); - writer.commit(commitData); + writer.setCommitData(commitData); + writer.commit(); // SolrCore.verbose("writer.commit() end"); numDocsPending.set(0); callPostCommitCallbacks(); @@ -707,7 +708,8 @@ // todo: refactor this shared code (or figure out why a real CommitUpdateCommand can't be used) final Map commitData = new HashMap(); commitData.put(SolrIndexWriter.COMMIT_TIME_MSEC_KEY, String.valueOf(System.currentTimeMillis())); - writer.commit(commitData); + writer.setCommitData(commitData); + writer.commit(); synchronized (solrCoreState.getUpdateLock()) { ulog.postCommit(cmd);