You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2013/08/14 14:07:36 UTC
svn commit: r1513825 - in /lucene/dev/trunk/solr: CHANGES.txt
core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java
Author: markrmiller
Date: Wed Aug 14 12:07:35 2013
New Revision: 1513825
URL: http://svn.apache.org/r1513825
Log:
SOLR-5134: Have HdfsIndexOutput extend BufferedIndexOutput.
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1513825&r1=1513824&r2=1513825&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Wed Aug 14 12:07:35 2013
@@ -120,13 +120,14 @@ Bug Fixes
* SOLR-5133: HdfsUpdateLog can fail to close a FileSystem instance if init
is called more than once. (Mark Miller)
-
Optimizations
----------------------
* SOLR-5044: Admin UI - Note on Core-Admin about directories while creating
core (steffkes)
+* SOLR-5134: Have HdfsIndexOutput extend BufferedIndexOutput. (Mark Miller)
+
Other Changes
----------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java?rev=1513825&r1=1513824&r2=1513825&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java Wed Aug 14 12:07:35 2013
@@ -23,18 +23,19 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.List;
-import org.apache.commons.io.IOUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataInputStream;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.lucene.store.BufferedIndexOutput;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.NoLockFactory;
import org.apache.solr.store.blockcache.CustomBufferedIndexInput;
+import org.apache.solr.util.IOUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -214,7 +215,7 @@ public class HdfsDirectory extends Direc
}
}
- static class HdfsIndexOutput extends IndexOutput {
+ static class HdfsIndexOutput extends BufferedIndexOutput {
private HdfsFileWriter writer;
@@ -224,33 +225,24 @@ public class HdfsDirectory extends Direc
@Override
public void close() throws IOException {
+ try {
+ super.close();
+ } catch (Throwable t) {
+ LOG.error("Error while closing", t);
+ }
writer.close();
}
-
- @Override
- public void flush() throws IOException {
- writer.flush();
- }
-
+
@Override
- public long getFilePointer() {
- return writer.getPosition();
+ protected void flushBuffer(byte[] b, int offset, int len)
+ throws IOException {
+ writer.writeBytes(b, offset, len);
}
-
+
@Override
- public long length() {
+ public long length() throws IOException {
return writer.length();
}
-
- @Override
- public void writeByte(byte b) throws IOException {
- writer.writeByte(b);
- }
-
- @Override
- public void writeBytes(byte[] b, int offset, int length) throws IOException {
- writer.writeBytes(b, offset, length);
- }
}
@Override
RE: svn commit: r1513825 - in /lucene/dev/trunk/solr: CHANGES.txt core/src/java/org/apache/solr/store/hdfs/HdfsDirectory.java
Posted by Uwe Schindler <uw...@thetaphi.de>.
Hi,
I am not sure if this is a good idea, unfortunately I have not seen that earlier:
> @Override
> public void close() throws IOException {
> + try {
> + super.close();
> + } catch (Throwable t) {
> + LOG.error("Error while closing", t);
> + }
> writer.close();
> }
Super.close() writes the final (not yet written) buffer to disk. If an error occurs, this is completely ignored, so IndexWriter would think that the data is written and would not fail to commit! The close of HDFS writer afterward will almost never fail, so it’s a completely swallowed write error. This is worse because BufferedIndexOutput is different than the old code, because close() acztuall does something! Also catching Throwable is not a good idea (think of ThreadInterruptedException!)
It would be better to clone the code from FSIndexOutput:
Import o.a.l.util.IOUtils;
public void close() throws IOException {
IOException priorE = null;
try {
super.close();
} catch (IOException ioe) {
priorE = ioe;
} finally {
IOUtils.closeWhileHandlingException(priorE, writer);
}
}
Uwe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org