Skip to content

Commit

Permalink
pool: add option to use direct-io while checksum calculation
Browse files Browse the repository at this point in the history 
Motivation:
When dcache pool runs full scrub, then it goes one through all files and
by reading invalidates filesystem cache. Thus, any competing user IO get
affected.

Modification:
Add option to scrub to enable/disable O_DIRECT to by-pass filesystem
cache during checksum calculation.

Result:
Less affect on user IO during scrub.

Acked-by: Paul Millar
Target: master
Require-book: yes
Require-notes: yes
  • Loading branch information
@kofemann
kofemann committed Nov 20, 2023
1 parent 0f098e8 commit 87f1964
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 14 deletions.
14 changes: 14 additions & 0 deletions docs/TheBook/src/main/markdown/cookbook-pool.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ If an option is enabled a checksum is calculated as described. If there is alrea
>
> Do not change the default configuration for the option `enforcecrc`. This option should always be enabled as this ensures that there will always be a checksum stored with a file.
#### Using direct-IO

When disk scrub is enabled or a checksum verification is triggered by admin, dCache pool
will sequentially read all files in the pool to calculate the checksums. Those reads going
through the filesystem cache and, eventually, evict data accessed by user applications via
NFS or xroot protocols, thus, application will experience high IO latency.

To avoid such situations, pool support disabling read-through filesystem cache by opening
file with `O_DIRECT` flag. This bahavoir can be controlled by:

csm use directio on|off

admin command. By default, the direct-IO capability is disabled.

## Migration Module

The purpose of the migration module is essentially to copy or move the content of a pool to one or more other pools.
Expand Down
81 changes: 67 additions & 14 deletions modules/dcache/src/main/java/org/dcache/pool/classic/ChecksumScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@
import static org.dcache.util.Exceptions.messageOrClassName;

import com.google.common.collect.Iterables;
import com.sun.nio.file.ExtendedOpenOption;
import diskCacheV111.util.CacheException;
import diskCacheV111.util.FileCorruptedCacheException;
import diskCacheV111.util.FileNotInCacheException;
import diskCacheV111.util.PnfsId;
import dmg.cells.nucleus.CellCommandListener;
import dmg.cells.nucleus.CellInfoProvider;
import dmg.cells.nucleus.CellLifeCycleAware;
import dmg.cells.nucleus.CellSetupProvider;
import dmg.util.CommandException;
import dmg.util.command.Argument;
import dmg.util.command.Command;
import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
Expand All @@ -26,8 +30,9 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
Expand All @@ -42,17 +47,11 @@
import org.slf4j.LoggerFactory;

public class ChecksumScanner
implements CellCommandListener, CellLifeCycleAware {
implements CellCommandListener, CellLifeCycleAware, CellSetupProvider, CellInfoProvider {

private static final Logger LOGGER =
LoggerFactory.getLogger(ChecksumScanner.class);

/**
* OpenOptions to use by scanner, when checksum of a file is calculated.
*/
private static final EnumSet<? extends OpenOption> SCANNER_OPEN_OPTIONS = EnumSet.of(
OpenFlags.NOATIME);

private final FullScan _fullScan = new FullScan();
private final Scrubber _scrubber = new Scrubber();
private final SingleScan _singleScan = new SingleScan();
Expand All @@ -63,6 +62,11 @@ public class ChecksumScanner

private File _scrubberStateFile;

/**
* Indicates whatever file should be opened with direct io mode to by-pass file system cache
*/
private volatile boolean _useDirectIO;

/**
* Errors found while running 'csm check'.
*/
Expand Down Expand Up @@ -121,8 +125,7 @@ public void runIt() throws Exception {
_bad.clear();

for (PnfsId id : _repository) {
try (ReplicaDescriptor handle = _repository.openEntry(id,
SCANNER_OPEN_OPTIONS)) {
try (ReplicaDescriptor handle = _repository.openEntry(id,getOpenOptions())) {
_csm.verifyChecksum(handle);
} catch (FileNotInCacheException e) {
/* It was removed before we could get it. No problem.
Expand Down Expand Up @@ -184,8 +187,7 @@ public synchronized void go(PnfsId pnfsId) {
public void runIt()
throws CacheException, InterruptedException, IOException, NoSuchAlgorithmException {
stopScrubber();
try (ReplicaDescriptor handle = _repository.openEntry(_pnfsId,
EnumSet.of(OpenFlags.NOATIME))) {
try (ReplicaDescriptor handle = _repository.openEntry(_pnfsId, getOpenOptions())) {
_actualChecksums = _csm.verifyChecksum(handle);
} catch (FileCorruptedCacheException e) {
_expectedChecksums = e.getExpectedChecksums().get();
Expand Down Expand Up @@ -430,8 +432,7 @@ private void scanFiles(PnfsId[] repository)
try {
if (_repository.getState(id) == ReplicaState.CACHED ||
_repository.getState(id) == ReplicaState.PRECIOUS) {
ReplicaDescriptor handle =
_repository.openEntry(id, SCANNER_OPEN_OPTIONS);
ReplicaDescriptor handle = _repository.openEntry(id, getOpenOptions());
try {
_csm.verifyChecksumWithThroughputLimit(handle);
} finally {
Expand Down Expand Up @@ -467,6 +468,18 @@ public String toString() {
}
}

/**
* Returns OpenOptions to use by scanner, when checksum of a file is calculated.
*/
private Set<OpenOption> getOpenOptions() {
Set<OpenOption> scrubOpenOptions = new HashSet<>();
scrubOpenOptions.add(OpenFlags.NOATIME);
if (_useDirectIO) {
scrubOpenOptions.add(ExtendedOpenOption.DIRECT);
}
return scrubOpenOptions;
}

private void invalidateCacheEntryAndSendAlarm(PnfsId id, FileCorruptedCacheException e) {
LOGGER.error(
AlarmMarkerFactory.getMarker(PredefinedAlarm.CHECKSUM, id.toString(), poolName),
Expand Down Expand Up @@ -596,6 +609,33 @@ public String call() {
}
}

@Command(name = "csm use directio",
hint = "enable/disable whether to bypass filesystem cache",
description = "If set to \"on,\" the scrubber will open files for checksum calculation using " +
"the O_DIRECT mode to bypass the filesystem cache. This approach may enhance client IO " +
"latency in certain scenarios but could potentially reduce the scrubbing throughput.")
public class CsmDirectioCommand implements Callable<String> {

@Argument(valueSpec = "on|off", usage = "Specify whether directio is used.")
String enable;

@Override
public String call() throws CommandException {

switch (enable.toLowerCase()) {
case "on":
_useDirectIO = true;
break;
case "off":
_useDirectIO = false;
break;
default:
throw new CommandException("Invalid value: " + enable);
}
return enable;
}
}

@Command(name = "csm show errors",
hint = "show errors found during checksum verification",
description = "Display the list of all errors found while running " +
Expand Down Expand Up @@ -630,4 +670,17 @@ public void beforeStop() {
_csm.removeListener(listener);
stopScrubber();
}

@Override
public synchronized void printSetup(PrintWriter pw) {
if (_useDirectIO) {
pw.println("csm use directio on");
}
}

@Override
public synchronized void getInfo(PrintWriter pw) {
pw.printf("use directio : %s\n", _useDirectIO ? "on" : "off");
}

}

0 comments on commit 87f1964

Please sign in to comment.