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 2015/02/08 19:49:27 UTC
svn commit: r1658240 - in /lucene/dev/branches/lucene_solr_5_0: ./ solr/
solr/core/ solr/core/src/java/org/apache/solr/handler/
solr/core/src/test/org/apache/solr/cloud/hdfs/
Author: markrmiller
Date: Sun Feb 8 18:49:26 2015
New Revision: 1658240
URL: http://svn.apache.org/r1658240
Log:
SOLR-6920, SOLR-6640: A replicated index can end up corrupted when small files end up with the same file name and size.
Modified:
lucene/dev/branches/lucene_solr_5_0/ (props changed)
lucene/dev/branches/lucene_solr_5_0/solr/ (props changed)
lucene/dev/branches/lucene_solr_5_0/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/lucene_solr_5_0/solr/core/ (props changed)
lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
lucene/dev/branches/lucene_solr_5_0/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java
Modified: lucene/dev/branches/lucene_solr_5_0/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_0/solr/CHANGES.txt?rev=1658240&r1=1658239&r2=1658240&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_0/solr/CHANGES.txt (original)
+++ lucene/dev/branches/lucene_solr_5_0/solr/CHANGES.txt Sun Feb 8 18:49:26 2015
@@ -489,6 +489,9 @@ Bug Fixes
* olap.* in AnalyticsComponent
(Alexandre Rafalovitch & hossman)
+* SOLR-6920: A replicated index can end up corrupted when small files end up with the same
+ file name and size. (Varun Thacker, Mark Miller)
+
Optimizations
----------------------
Modified: lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java?rev=1658240&r1=1658239&r2=1658240&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java (original)
+++ lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java Sun Feb 8 18:49:26 2015
@@ -23,7 +23,6 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.OutputStream;
-import java.io.Writer;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
@@ -34,7 +33,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
@@ -46,15 +44,18 @@ import java.util.zip.Checksum;
import java.util.zip.DeflaterOutputStream;
import org.apache.commons.io.IOUtils;
+import org.apache.lucene.codecs.CodecUtil;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexCommit;
import org.apache.lucene.index.IndexDeletionPolicy;
import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.SegmentCommitInfo;
+import org.apache.lucene.index.SegmentInfos;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.RateLimiter;
-import org.apache.solr.client.solrj.impl.BinaryResponseParser;
+import org.apache.lucene.util.Version;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.params.CommonParams;
@@ -72,7 +73,6 @@ import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrDeletionPolicy;
import org.apache.solr.core.SolrEventListener;
import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.response.BinaryQueryResponseWriter;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.update.SolrIndexWriter;
@@ -427,28 +427,68 @@ public class ReplicationHandler extends
core.getDeletionPolicy().setReserveDuration(gen, reserveCommitDuration);
List<Map<String, Object>> result = new ArrayList<>();
Directory dir = null;
+ Version version = null;
try {
- // get all the files in the commit
- // use a set to workaround possible Lucene bug which returns same file
- // name multiple times
- Collection<String> files = new HashSet<>(commit.getFileNames());
dir = core.getDirectoryFactory().get(core.getNewIndexDir(), DirContext.DEFAULT, core.getSolrConfig().indexConfig.lockType);
- try {
-
- for (String fileName : files) {
- if (fileName.endsWith(".lock")) continue;
+ SegmentInfos infos = SegmentInfos.readCommit(dir, commit.getSegmentsFileName());
+ Version oldestVersion = Version.LUCENE_CURRENT;
+ for (SegmentCommitInfo commitInfo : infos) {
+ version = commitInfo.info.getVersion();
+ if (oldestVersion.onOrAfter(version)) {
+ oldestVersion = version;
+ }
+ for (String file : commitInfo.files()) {
Map<String,Object> fileMeta = new HashMap<>();
- fileMeta.put(NAME, fileName);
- fileMeta.put(SIZE, dir.fileLength(fileName));
+ fileMeta.put(NAME, file);
+ fileMeta.put(SIZE, dir.fileLength(file));
+
+ try (final IndexInput in = dir.openInput(file, IOContext.READONCE)) {
+ if (version.onOrAfter(Version.LUCENE_4_8_0)) {
+ try {
+ long checksum = CodecUtil.retrieveChecksum(in);
+ fileMeta.put(CHECKSUM, checksum);
+ } catch(Exception e) {
+ LOG.warn("Could not read checksum from index file.", e);
+ }
+ }
+ }
+
result.add(fileMeta);
}
- } finally {
- core.getDirectoryFactory().release(dir);
}
+
+ // add the segments_N file
+
+ // we use the oldest version seen to determine
+ // how we treat the segments_N file - conservative, easy
+
+ Map<String,Object> fileMeta = new HashMap<>();
+ fileMeta.put(NAME, infos.getSegmentsFileName());
+ fileMeta.put(SIZE, dir.fileLength(infos.getSegmentsFileName()));
+ if (infos.getId() != null) {
+ try (final IndexInput in = dir.openInput(infos.getSegmentsFileName(), IOContext.READONCE)) {
+ if (oldestVersion.onOrAfter(Version.LUCENE_4_8_0)) {
+ try {
+ fileMeta.put(CHECKSUM, CodecUtil.retrieveChecksum(in));
+ } catch(Exception e) {
+ LOG.warn("Could not read checksum from index file.", e);
+ }
+ }
+ }
+ }
+ result.add(fileMeta);
} catch (IOException e) {
rsp.add("status", "unable to get file names for given index generation");
rsp.add("exception", e);
LOG.error("Unable to get file names for indexCommit generation: " + gen, e);
+ } finally {
+ if (dir != null) {
+ try {
+ core.getDirectoryFactory().release(dir);
+ } catch (IOException e) {
+ SolrException.log(LOG, "Could not release directory after fetching file list", e);
+ }
+ }
}
rsp.add(CMD_GET_FILE_LIST, result);
if (confFileNameAlias.size() < 1 || core.getCoreDescriptor().getCoreContainer().isZooKeeperAware())
Modified: lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/SnapPuller.java?rev=1658240&r1=1658239&r2=1658240&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/SnapPuller.java (original)
+++ lucene/dev/branches/lucene_solr_5_0/solr/core/src/java/org/apache/solr/handler/SnapPuller.java Sun Feb 8 18:49:26 2015
@@ -78,6 +78,7 @@ import java.util.zip.InflaterInputStream
import org.apache.commons.io.IOUtils;
import org.apache.http.client.HttpClient;
+import org.apache.lucene.codecs.CodecUtil;
import org.apache.lucene.index.IndexCommit;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.SegmentCommitInfo;
@@ -86,6 +87,7 @@ 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.util.Version;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
@@ -347,7 +349,6 @@ public class SnapPuller {
}
}
-
if (latestVersion == 0L) {
if (forceReplication && commit.getGeneration() != 0) {
// since we won't get the files for an empty index,
@@ -404,7 +405,18 @@ public class SnapPuller {
try {
- if (isIndexStale(indexDir)) {
+ SegmentInfos infos = SegmentInfos.readCommit(indexDir, commit.getSegmentsFileName());
+
+ // we treat these files as if they all the oldest version we see
+ Version oldestVersion = Version.LUCENE_CURRENT;
+ for (SegmentCommitInfo commitInfo : infos) {
+ Version version = commitInfo.info.getVersion();
+ if (oldestVersion.onOrAfter(version)) {
+ oldestVersion = version;
+ }
+ }
+
+ if (isIndexStale(indexDir, oldestVersion)) {
isFullCopyNeeded = true;
}
@@ -451,8 +463,7 @@ public class SnapPuller {
+ isFullCopyNeeded);
successfulInstall = false;
- downloadIndexFiles(isFullCopyNeeded, indexDir, tmpIndexDir,
- latestGeneration);
+ downloadIndexFiles(isFullCopyNeeded, indexDir, oldestVersion, tmpIndexDir, latestGeneration);
LOG.info("Total time taken for download : "
+ ((System.currentTimeMillis() - replicationStartTime) / 1000)
+ " secs");
@@ -803,19 +814,22 @@ public class SnapPuller {
* Download the index files. If a new index is needed, download all the files.
*
* @param downloadCompleteIndex is it a fresh index copy
- * @param tmpIndexDir the directory to which files need to be downloadeed to
* @param indexDir the indexDir to be merged to
+ * @param version Version of index
+ * @param tmpIndexDir the directory to which files need to be downloadeed to
* @param latestGeneration the version number
*/
- private void downloadIndexFiles(boolean downloadCompleteIndex,
- Directory indexDir, Directory tmpIndexDir, long latestGeneration)
+ private void downloadIndexFiles(boolean downloadCompleteIndex, Directory indexDir, Version version, Directory tmpIndexDir, long latestGeneration)
throws Exception {
if (LOG.isDebugEnabled()) {
LOG.debug("Download files to dir: " + Arrays.asList(indexDir.listAll()));
}
for (Map<String,Object> file : filesToDownload) {
- if (!slowFileExists(indexDir, (String) file.get(NAME))
- || downloadCompleteIndex) {
+ String filename = (String) file.get(NAME);
+ long size = (Long) file.get(SIZE);
+ CompareResult compareResult = compareFile(indexDir, version, filename, size, (Long) file.get(CHECKSUM));
+ if (!compareResult.equal || downloadCompleteIndex
+ || filesToAlwaysDownloadIfNoChecksums(filename, size, compareResult)) {
dirFileFetcher = new DirectoryFileFetcher(tmpIndexDir, file,
(String) file.get(NAME), false, latestGeneration);
currentFile = file;
@@ -828,6 +842,74 @@ public class SnapPuller {
}
}
+ private boolean filesToAlwaysDownloadIfNoChecksums(String filename,
+ long size, CompareResult compareResult) {
+ // without checksums to compare, we always download .si, .liv, segments_N,
+ // and any file under 100kb
+ return !compareResult.checkSummed && (filename.endsWith(".si") || filename.endsWith(".liv")
+ || filename.startsWith("segments_") || size < 100000);
+ }
+
+ static class CompareResult {
+ boolean equal = false;
+ boolean checkSummed = false;
+ }
+
+ private CompareResult compareFile(Directory indexDir, Version version, String filename, Long backupIndexFileLen, Long backupIndexFileChecksum) {
+ CompareResult compareResult = new CompareResult();
+ try {
+ try (final IndexInput indexInput = indexDir.openInput(filename, IOContext.READONCE)) {
+ long indexFileLen = indexInput.length();
+ long indexFileChecksum = 0;
+
+ if (backupIndexFileChecksum != null && version.onOrAfter(Version.LUCENE_4_8_0)) {
+ try {
+ indexFileChecksum = CodecUtil.retrieveChecksum(indexInput);
+ compareResult.checkSummed = true;
+ } catch (Exception e) {
+ LOG.warn("Could not retrieve checksum from file.", e);
+ }
+ }
+
+ if (!compareResult.checkSummed) {
+ // we don't have checksums to compare
+
+ if (indexFileLen == backupIndexFileLen) {
+ compareResult.equal = true;
+ return compareResult;
+ } else {
+ LOG.warn(
+ "File {} did not match. " + "expected length is {} and actual length is {}",
+ filename, backupIndexFileChecksum, indexFileChecksum,
+ backupIndexFileLen, indexFileLen);
+ compareResult.equal = false;
+ return compareResult;
+ }
+ }
+
+ // we have checksums to compare
+
+ if (indexFileLen == backupIndexFileLen && indexFileChecksum == backupIndexFileChecksum) {
+ compareResult.equal = true;
+ return compareResult;
+ } else {
+ LOG.warn("File {} did not match. expected checksum is {} and actual is checksum {}. " +
+ "expected length is {} and actual length is {}", filename, backupIndexFileChecksum, indexFileChecksum,
+ backupIndexFileLen, indexFileLen);
+ compareResult.equal = false;
+ return compareResult;
+ }
+ }
+ } catch (NoSuchFileException | FileNotFoundException e) {
+ compareResult.equal = false;
+ return compareResult;
+ } catch (IOException e) {
+ LOG.error("Could not read file " + filename + ". Downloading it again", e);
+ compareResult.equal = false;
+ return compareResult;
+ }
+ }
+
/** Returns true if the file exists (can be opened), false
* if it cannot be opened, and (unlike Java's
* File.exists) throws IOException if there's some
@@ -848,15 +930,24 @@ public class SnapPuller {
* @return true if the index stale and we need to download a fresh copy, false otherwise.
* @throws IOException if low level io error
*/
- private boolean isIndexStale(Directory dir) throws IOException {
+ private boolean isIndexStale(Directory dir, Version indexVersion) throws IOException {
for (Map<String, Object> file : filesToDownload) {
- if (slowFileExists(dir, (String) file.get(NAME))
- && dir.fileLength((String) file.get(NAME)) != (Long) file.get(SIZE)) {
- LOG.warn("File " + file.get(NAME) + " expected to be " + file.get(SIZE)
- + " while it is " + dir.fileLength((String) file.get(NAME)));
- // file exists and size is different, therefore we must assume
- // corrupted index
- return true;
+ String filename = (String) file.get(NAME);
+ Long length = (Long) file.get(SIZE);
+ Long checksum = (Long) file.get(CHECKSUM);
+ if (slowFileExists(dir, filename)) {
+ if (checksum != null) {
+ if (!(compareFile(dir, indexVersion, filename, length, checksum).equal)) {
+ // file exists and size or checksum is different, therefore we must download it again
+ return true;
+ }
+ } else {
+ if (length != dir.fileLength(filename)) {
+ LOG.warn("File {} did not match. expected length is {} and actual length is {}",
+ filename, length, dir.fileLength(filename));
+ return true;
+ }
+ }
}
}
return false;
Modified: lucene/dev/branches/lucene_solr_5_0/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_0/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java?rev=1658240&r1=1658239&r2=1658240&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_0/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java (original)
+++ lucene/dev/branches/lucene_solr_5_0/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java Sun Feb 8 18:49:26 2015
@@ -138,7 +138,7 @@ public class HdfsTestUtil {
String dir = uri.toString()
+ "/"
+ new File(dataDir).toString().replaceAll(":", "_")
- .replaceAll("/", "_");
+ .replaceAll("/", "_").replaceAll(" ", "_");
return dir;
}