Skip to content
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

Jump to bottom

More detailed tracing when writing metadata #31319

Merged
merged 2 commits into from
Jun 14, 2018
Merged

More detailed tracing when writing metadata #31319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6009dc3
More detailed tracing during writing metadata
DaveCTurner Jun 14, 2018
4fc8a94
Trace deleteIfExists calls too
DaveCTurner Jun 14, 2018
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
21 changes: 13 additions & 8 deletions server/src/main/java/org/elasticsearch/gateway/MetaDataStateFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.OutputStreamIndexOutput;
import org.apache.lucene.store.SimpleFSDirectory;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.bytes.BytesArray;
Expand Down Expand Up @@ -76,6 +77,7 @@ public abstract class MetaDataStateFormat<T> {
private final String prefix;
private final Pattern stateFilePattern;

private static final Logger logger = Loggers.getLogger(MetaDataStateFormat.class);

/**
* Creates a new {@link MetaDataStateFormat} instance
Expand Down Expand Up @@ -134,6 +136,7 @@ public void close() throws IOException {
IOUtils.fsync(tmpStatePath, false); // fsync the state file
Files.move(tmpStatePath, finalStatePath, StandardCopyOption.ATOMIC_MOVE);
IOUtils.fsync(stateLocation, true);
logger.trace("written state to {}", finalStatePath);
for (int i = 1; i < locations.length; i++) {
stateLocation = locations[i].resolve(STATE_DIR_NAME);
Files.createDirectories(stateLocation);
Expand All @@ -145,12 +148,15 @@ public void close() throws IOException {
// we are on the same FileSystem / Partition here we can do an atomic move
Files.move(tmpPath, finalPath, StandardCopyOption.ATOMIC_MOVE);
IOUtils.fsync(stateLocation, true);
logger.trace("copied state to {}", finalPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to do this after the deleteIfExists? for the odd case that deletes are slow?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about tracing the cleanupOldFiles method run.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. It seems unlikely since these calls don't fsync(), but there's little harm in checking. I pushed 4fc8a94.

} finally {
Files.deleteIfExists(tmpPath);
logger.trace("cleaned up {}", tmpPath);
}
}
} finally {
Files.deleteIfExists(tmpStatePath);
logger.trace("cleaned up {}", tmpStatePath);
}
cleanupOldFiles(prefix, fileName, locations);
}
Expand Down Expand Up @@ -211,20 +217,19 @@ protected Directory newDirectory(Path dir) throws IOException {
}

private void cleanupOldFiles(final String prefix, final String currentStateFile, Path[] locations) throws IOException {
final DirectoryStream.Filter<Path> filter = new DirectoryStream.Filter<Path>() {
@Override
public boolean accept(Path entry) throws IOException {
final String entryFileName = entry.getFileName().toString();
return Files.isRegularFile(entry)
&& entryFileName.startsWith(prefix) // only state files
&& currentStateFile.equals(entryFileName) == false; // keep the current state file around
}
final DirectoryStream.Filter<Path> filter = entry -> {
final String entryFileName = entry.getFileName().toString();
return Files.isRegularFile(entry)
&& entryFileName.startsWith(prefix) // only state files
&& currentStateFile.equals(entryFileName) == false; // keep the current state file around
};
// now clean up the old files
for (Path dataLocation : locations) {
logger.trace("cleanupOldFiles: cleaning up {}", dataLocation);
try (DirectoryStream<Path> stream = Files.newDirectoryStream(dataLocation.resolve(STATE_DIR_NAME), filter)) {
for (Path stateFile : stream) {
Files.deleteIfExists(stateFile);
logger.trace("cleanupOldFiles: cleaned up {}", stateFile);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/gateway/MetaStateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public void writeIndex(String reason, IndexMetaData indexMetaData) throws IOExce
try {
IndexMetaData.FORMAT.write(indexMetaData,
nodeEnv.indexPaths(indexMetaData.getIndex()));
logger.trace("[{}] state written", index);
} catch (Exception ex) {
logger.warn(() -> new ParameterizedMessage("[{}]: failed to write index state", index), ex);
throw new IOException("failed to write state for [" + index + "]", ex);
Expand All @@ -136,6 +137,7 @@ void writeGlobalState(String reason, MetaData metaData) throws IOException {
logger.trace("[_global] writing state, reason [{}]", reason);
try {
MetaData.FORMAT.write(metaData, nodeEnv.nodeDataPaths());
logger.trace("[_global] state written");
} catch (Exception ex) {
logger.warn("[_global]: failed to write global state", ex);
throw new IOException("failed to write global state", ex);
Expand Down