Skip to content

SOLR-18121: Rm 'rm-in-advance' fetcher codepath #4258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gerlowskija wants to merge 1 commit into
base: main
Choose a base branch
from
Open

SOLR-18121: Rm 'rm-in-advance' fetcher codepath #4258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions solr/core/src/java/org/apache/solr/core/SolrCore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,6 @@ public class SolrCore implements SolrInfoBean, Closeable {
private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
private final SolrMetricsContext solrMetricsContext;

public volatile boolean searchEnabled = true;
public volatile boolean indexEnabled = true;
public volatile boolean readOnly = false;

private PackageListeners packageListeners = new PackageListeners(this);
Expand Down Expand Up @@ -2143,11 +2141,7 @@ boolean areAllSearcherReferencesEmpty() {
* @see #withSearcher(IOFunction)
*/
public RefCounted<SolrIndexSearcher> getSearcher() {
if (searchEnabled) {
return getSearcher(false, true, null);
}
throw new SolrException(
SolrException.ErrorCode.SERVICE_UNAVAILABLE, "Search is temporarily disabled");
return getSearcher(false, true, null);
}

/**
Expand Down
102 changes: 1 addition & 101 deletions solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileStore;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
Expand All @@ -72,7 +71,6 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.function.BooleanSupplier;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.zip.Adler32;
import java.util.zip.Checksum;
Expand Down Expand Up @@ -185,8 +183,6 @@ public class IndexFetcher {

private boolean skipCommitOnLeaderVersionZero = true;

private boolean clearLocalIndexFirst = false;

private static final String INTERRUPT_RESPONSE_MESSAGE =
"Interrupted while waiting for modify lock";

Expand Down Expand Up @@ -409,7 +405,6 @@ IndexFetchResult fetchLatestIndex(boolean forceReplication)
IndexFetchResult fetchLatestIndex(boolean forceReplication, boolean forceCoreReload)
throws IOException, InterruptedException {

this.clearLocalIndexFirst = false;
boolean cleanupDone = false;
boolean successfulInstall = false;
markReplicationStart();
Expand Down Expand Up @@ -697,9 +692,7 @@ IndexFetchResult fetchLatestIndex(boolean forceReplication, boolean forceCoreRel
// let the system know we are changing dir's and the old one
// may be closed
if (indexDir != null) {
if (!this.clearLocalIndexFirst) { // it was closed earlier
solrCore.getDirectoryFactory().doneWithDirectory(indexDir);
}
solrCore.getDirectoryFactory().doneWithDirectory(indexDir);
// Cleanup all index files not associated with any *named* snapshot.
solrCore.deleteNonSnapshotIndexFiles(indexDirPath);
}
Expand All @@ -722,8 +715,6 @@ IndexFetchResult fetchLatestIndex(boolean forceReplication, boolean forceCoreRel
}
}
} finally {
solrCore.searchEnabled = true;
solrCore.indexEnabled = true;
if (!isFullCopyNeeded) {
solrCore.getUpdateHandler().getSolrCoreState().openIndexWriter(solrCore);
}
Expand Down Expand Up @@ -927,9 +918,6 @@ private void logReplicationTimeAndConfFiles(
props.setProperty(INDEX_REPLICATED_AT, String.valueOf(replicationTime));
props.setProperty(PREVIOUS_CYCLE_TIME_TAKEN, String.valueOf(replicationTimeTaken));
props.setProperty(TIMES_INDEX_REPLICATED, String.valueOf(indexCount));
if (clearLocalIndexFirst) {
props.setProperty(CLEARED_LOCAL_IDX, "true");
}
if (modifiedConfFiles != null && !modifiedConfFiles.isEmpty()) {
props.setProperty(CONF_FILES_REPLICATED, confFiles.toString());
props.setProperty(CONF_FILES_REPLICATED_AT, String.valueOf(replicationTime));
Expand Down Expand Up @@ -1128,22 +1116,12 @@ private long downloadIndexFiles(
|| (tmpIndexDir instanceof FilterDirectory
&& FilterDirectory.unwrap(tmpIndexDir) instanceof FSDirectory));

long totalSpaceRequired = 0;
for (Map<String, Object> file : filesToDownload) {
long size = (Long) file.get(SIZE);
totalSpaceRequired += size;
}

if (log.isInfoEnabled()) {
log.info(
"tmpIndexDir_type : {} , {}",
tmpIndexDir.getClass(),
FilterDirectory.unwrap(tmpIndexDir));
}
long usableSpace = usableDiskSpaceProvider.apply(tmpIndexDirPath);
if (getApproxTotalSpaceReqd(totalSpaceRequired) > usableSpace) {
deleteFilesInAdvance(indexDir, indexDirPath, totalSpaceRequired, usableSpace);
}

for (Map<String, Object> file : filesToDownload) {
String filename = (String) file.get(NAME);
Expand Down Expand Up @@ -1201,85 +1179,9 @@ private long downloadIndexFiles(
// only for testing purposes. do not use this anywhere else
// -----------START----------------------
static BooleanSupplier testWait = () -> true;
static Function<String, Long> usableDiskSpaceProvider = dir -> getUsableSpace(dir);

// ------------ END---------------------

private static Long getUsableSpace(String dir) {
try {
Path file = Path.of(dir);
if (Files.notExists(file)) {
file = file.getParent();
// this is not a disk directory. so just pretend that there is enough space
if (Files.notExists(file)) {
return Long.MAX_VALUE;
}
}
FileStore fileStore = Files.getFileStore(file);
return fileStore.getUsableSpace();
} catch (IOException e) {
throw new SolrException(ErrorCode.SERVER_ERROR, "Could not free disk space", e);
}
}

private long getApproxTotalSpaceReqd(long totalSpaceRequired) {
long approxTotalSpaceReqd = (long) (totalSpaceRequired * 1.05); // add 5% extra for safety
// we should have an extra of 100MB free after everything is downloaded
approxTotalSpaceReqd += (100 * 1024 * 1024);
return approxTotalSpaceReqd;
}

private void deleteFilesInAdvance(
Directory indexDir, String indexDirPath, long usableDiskSpace, long totalSpaceRequired)
throws IOException {
long actualSpaceReqd = totalSpaceRequired;
List<String> filesTobeDeleted = new ArrayList<>();
long clearedSpace = 0;
// go through each file to check if this needs to be deleted
for (String f : indexDir.listAll()) {
for (Map<String, Object> fileInfo : filesToDownload) {
if (f.equals(fileInfo.get(NAME))) {
String filename = (String) fileInfo.get(NAME);
long size = (Long) fileInfo.get(SIZE);
CompareResult compareResult =
compareFile(indexDir, filename, size, (Long) fileInfo.get(CHECKSUM));
if (!compareResult.equal || filesToAlwaysDownloadIfNoChecksums(f, size, compareResult)) {
filesTobeDeleted.add(f);
clearedSpace += size;
} else {
/*this file will not be downloaded*/
actualSpaceReqd -= size;
}
}
}
}
if (usableDiskSpace > getApproxTotalSpaceReqd(actualSpaceReqd)) {
// after considering the files actually available locally we really don't need to do any
// delete
return;
}
log.info(
"This disk does not have enough space to download the index from leader. So cleaning up the local index. "
+ " This may lead to loss of data/or node if index replication fails in between");
// now we should disable searchers and index writers because this core will not have all the
// required files
this.clearLocalIndexFirst = true;
this.solrCore.searchEnabled = false;
this.solrCore.indexEnabled = false;
solrCore.getDirectoryFactory().doneWithDirectory(indexDir);
solrCore.deleteNonSnapshotIndexFiles(indexDirPath);
this.solrCore.closeSearcher();
assert testWait.getAsBoolean();
solrCore.getUpdateHandler().getSolrCoreState().closeIndexWriter(this.solrCore, false);
for (String f : filesTobeDeleted) {
try {
indexDir.deleteFile(f);
} catch (FileNotFoundException | NoSuchFileException e) {
// no problem , it was deleted by someone else
}
}
}

static boolean filesToAlwaysDownloadIfNoChecksums(
String filename, long size, CompareResult compareResult) {
// without checksums to compare, we always download .si, .liv, segments_N,
Expand Down Expand Up @@ -2061,8 +1963,6 @@ String getLeaderCoreUrl() {

static final String TIMES_INDEX_REPLICATED = "timesIndexReplicated";

static final String CLEARED_LOCAL_IDX = "clearedLocalIndexFirst";

static final String CONF_FILES_REPLICATED = "confFilesReplicated";

static final String CONF_FILES_REPLICATED_AT = "confFilesReplicatedAt";
Expand Down
1 change: 0 additions & 1 deletion solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,6 @@ private void addReplicationProperties(BiConsumer<String, Object> consumer, Prope
addVal(consumer, IndexFetcher.TIMES_FAILED, props, Integer.class);
addVal(consumer, IndexFetcher.REPLICATION_FAILED_AT, props, Date.class);
addVal(consumer, IndexFetcher.PREVIOUS_CYCLE_TIME_TAKEN, props, Long.class);
addVal(consumer, IndexFetcher.CLEARED_LOCAL_IDX, props, Boolean.class);
}

private void addVal(
Expand Down
2 changes: 1 addition & 1 deletion solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private void closeIndexWriter(IndexWriterCloser closer) {
@Override
public RefCounted<IndexWriter> getIndexWriter(SolrCore core, boolean failOnReadOnly)
throws IOException {
if (core != null && (!core.indexEnabled || (core.readOnly && failOnReadOnly))) {
if (core != null && (core.readOnly && failOnReadOnly)) {
throw new SolrException(
SolrException.ErrorCode.SERVICE_UNAVAILABLE, "Indexing is temporarily disabled");
}
Expand Down
Loading
Loading