8257215: JFR: Events dropped when streaming over a chunk rotation

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2021-02-01 05:57:08 +00:00
parent 0fdf9cdd3a
commit 21f8bf4411
14 changed files with 167 additions and 85 deletions

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -41,6 +41,7 @@ import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
@ -851,4 +852,18 @@ public final class PlatformRecording implements AutoCloseable {
}
}
public void removePath(SafePath path) {
synchronized (recorder) {
Iterator<RepositoryChunk> it = chunks.iterator();
while (it.hasNext()) {
RepositoryChunk c = it.next();
if (c.getFile().equals(path)) {
it.remove();
removed(c);
return;
}
}
}
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -33,6 +33,7 @@ import java.util.HashSet;
import java.util.Set;
import jdk.jfr.internal.SecuritySupport.SafePath;
import jdk.jfr.internal.management.ChunkFilename;
public final class Repository {
@ -45,6 +46,7 @@ public final class Repository {
private final Set<SafePath> cleanupDirectories = new HashSet<>();
private SafePath baseLocation;
private SafePath repository;
private ChunkFilename chunkFilename;
private Repository() {
}
@ -61,6 +63,7 @@ public final class Repository {
// Probe to see if repository can be created, needed for fail fast
// during JVM startup or JFR.configure
this.repository = createRepository(baseLocation);
this.chunkFilename = null;
try {
// Remove so we don't "leak" repositories, if JFR is never started
// and shutdown hook not added.
@ -84,8 +87,13 @@ public final class Repository {
jvm.setRepositoryLocation(repository.toString());
SecuritySupport.setProperty(JFR_REPOSITORY_LOCATION_PROPERTY, repository.toString());
cleanupDirectories.add(repository);
chunkFilename = null;
}
return new RepositoryChunk(repository, timestamp);
if (chunkFilename == null) {
chunkFilename = ChunkFilename.newPriviliged(repository.toPath());
}
String filename = chunkFilename.next(timestamp.toLocalDateTime());
return new RepositoryChunk(new SafePath(filename), timestamp.toInstant());
} catch (Exception e) {
String errorMsg = String.format("Could not create chunk in repository %s, %s: %s", repository, e.getClass(), e.getMessage());
Logger.log(LogTag.JFR, LogLevel.ERROR, errorMsg);

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -47,7 +47,6 @@ final class RepositoryChunk {
}
};
private final SafePath repositoryPath;
private final SafePath chunkFile;
private final Instant startTime;
private final RandomAccessFile unFinishedRAF;
@ -56,28 +55,12 @@ final class RepositoryChunk {
private int refCount = 0;
private long size;
RepositoryChunk(SafePath path, ZonedDateTime timestamp) throws Exception {
this.startTime = timestamp.toInstant();
this.repositoryPath = path;
this.chunkFile = findFileName(repositoryPath, timestamp.toLocalDateTime());
RepositoryChunk(SafePath path, Instant startTime) throws Exception {
this.startTime = startTime;
this.chunkFile = path;
this.unFinishedRAF = SecuritySupport.createRandomAccessFile(chunkFile);
}
private static SafePath findFileName(SafePath directory, LocalDateTime time) throws Exception {
String filename = Utils.formatDateTime(time);
Path p = directory.toPath().resolve(filename + FILE_EXTENSION);
for (int i = 1; i < MAX_CHUNK_NAMES; i++) {
SafePath s = new SafePath(p);
if (!SecuritySupport.exists(s)) {
return s;
}
String extendedName = String.format("%s_%02d%s", filename, i, FILE_EXTENSION);
p = directory.toPath().resolve(extendedName);
}
p = directory.toPath().resolve(filename + "_" + System.currentTimeMillis() + FILE_EXTENSION);
return new SafePath(p);
}
void finish(Instant endTime) {
try {
finishWithException(endTime);

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -500,6 +500,11 @@ public final class SecuritySupport {
public long fileSize(Path p) throws IOException {
return doPrivilegedIOWithReturn( () -> Files.size(p));
}
@Override
public boolean exists(Path p) throws IOException {
return doPrivilegedIOWithReturn( () -> Files.exists(p));
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -184,7 +184,7 @@ public final class Utils {
// This method reduces the number of loaded classes
// compared to DateTimeFormatter
static String formatDateTime(LocalDateTime time) {
public static String formatDateTime(LocalDateTime time) {
StringBuilder sb = new StringBuilder(19);
sb.append(time.getYear() / 100);
appendPadded(sb, time.getYear() % 100, true);

@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -46,6 +46,8 @@ public abstract class FileAccess {
public abstract long fileSize(Path p) throws IOException;
public abstract boolean exists(Path s) throws IOException;
private static class UnPrivileged extends FileAccess {
@Override
public RandomAccessFile openRAF(File f, String mode) throws IOException {
@ -71,5 +73,10 @@ public abstract class FileAccess {
public long fileSize(Path p) throws IOException {
return Files.size(p);
}
@Override
public boolean exists(Path p) {
return Files.exists(p);
}
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -119,8 +119,7 @@ public final class OngoingStream extends EventByteStream {
throw new IOException("No progress");
}
startTimeNanos += header.getDurationNanos();
Instant timestamp = Utils.epochNanosToInstant(startTimeNanos);
ManagementSupport.removeBefore(recording, timestamp);
ManagementSupport.removePath(recording, path);
closeInput();
} else {
header.refresh();

@ -0,0 +1,90 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package jdk.jfr.internal.management;
import java.nio.file.Paths;
import java.nio.file.Path;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import jdk.jfr.internal.SecuritySupport;
import jdk.jfr.internal.SecuritySupport.SafePath;
import jdk.jfr.internal.Utils;
import jdk.jfr.internal.consumer.FileAccess;
// Allows a remote streaming client to create chunk files
// with same naming scheme as the JVM.
public final class ChunkFilename {
private static final int MAX_CHUNK_NAMES = 100_000;
private static final String FILE_EXTENSION = ".jfr";
private final Path directory;
private final FileAccess fileAcess;
private Path lastPath;
private int counter;
public static ChunkFilename newUnpriviliged(Path directory) {
return new ChunkFilename(directory, FileAccess.UNPRIVILEGED);
}
public static ChunkFilename newPriviliged(Path directory) {
return new ChunkFilename(directory, SecuritySupport.PRIVILEGED);
}
private ChunkFilename(Path directory, FileAccess fileAccess) {
// Avoid malicious implementations of Path interface
this.directory = Paths.get(directory.toString());
this.fileAcess = fileAccess;
}
public String next(LocalDateTime time) throws IOException {
String filename = Utils.formatDateTime(time);
Path p = directory.resolve(filename + FILE_EXTENSION);
// If less than one file per second (typically case)
if (lastPath == null || !p.equals(lastPath)) {
if (!fileAcess.exists(p)) {
counter = 1; // reset counter
lastPath = p;
return p.toString();
}
}
// If more than one file per second
while (counter < MAX_CHUNK_NAMES) {
String extendedName = String.format("%s_%02d%s", filename, counter, FILE_EXTENSION);
p = directory.resolve(extendedName);
counter++;
if (!fileAcess.exists(p)) {
return p.toString();
}
}
throw new IOException("Unable to find unused filename after " + counter + " attempts");
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -49,6 +49,7 @@ import jdk.jfr.internal.Logger;
import jdk.jfr.internal.MetadataRepository;
import jdk.jfr.internal.PlatformRecording;
import jdk.jfr.internal.PrivateAccess;
import jdk.jfr.internal.SecuritySupport.SafePath;
import jdk.jfr.internal.Utils;
import jdk.jfr.internal.WriteableUserPath;
import jdk.jfr.internal.consumer.EventDirectoryStream;
@ -141,7 +142,12 @@ public final class ManagementSupport {
public static void removeBefore(Recording recording, Instant timestamp) {
PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
pr.removeBefore(timestamp);
}
// Needed callback to detect when a chunk has been parsed.
public static void removePath(Recording recording, Path path) {
PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
pr.removePath(new SafePath(path));
}
// Needed callback to detect when a chunk has been parsed.

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -30,6 +30,7 @@ import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDateTime;
@ -40,19 +41,20 @@ import java.util.Deque;
import java.util.Iterator;
import java.util.Objects;
import jdk.jfr.internal.management.ChunkFilename;
import jdk.jfr.internal.management.ManagementSupport;
final class DiskRepository implements Closeable {
final static class DiskChunk {
final Path path;
final Instant startTime;
final long startTimeNanos;
Instant endTime;
long size;
DiskChunk(Path path, long startNanos) {
this.path = path;
this.startTime = ManagementSupport.epochNanosToInstant(startNanos);
this.startTimeNanos = startNanos;
}
}
@ -80,6 +82,7 @@ final class DiskRepository implements Closeable {
private final boolean deleteDirectory;
private final ByteBuffer buffer = ByteBuffer.allocate(256);
private final Path directory;
private final ChunkFilename chunkFilename;
private RandomAccessFile raf;
private RandomAccessFile previousRAF;
@ -103,6 +106,7 @@ final class DiskRepository implements Closeable {
public DiskRepository(Path path, boolean deleteDirectory) throws IOException {
this.directory = path;
this.deleteDirectory = deleteDirectory;
this.chunkFilename = ChunkFilename.newUnpriviliged(path);
}
public synchronized void write(byte[] bytes) throws IOException {
@ -295,8 +299,8 @@ final class DiskRepository implements Closeable {
previousRAFstate = state;
currentChunk.size = Files.size(currentChunk.path);
long durationNanos = buffer.getLong(HEADER_FILE_DURATION);
Duration d = Duration.ofNanos(durationNanos);
currentChunk.endTime = currentChunk.startTime.plus(d);
long endTimeNanos = currentChunk.startTimeNanos + durationNanos;
currentChunk.endTime = ManagementSupport.epochNanosToInstant(endTimeNanos);
}
raf.seek(position);
}
@ -325,44 +329,8 @@ final class DiskRepository implements Closeable {
int nanoOfSecond = (int) (nanos % 1_000_000_000);
ZoneOffset z = OffsetDateTime.now().getOffset();
LocalDateTime d = LocalDateTime.ofEpochSecond(epochSecond, nanoOfSecond, z);
String filename = formatDateTime(d);
Path p1 = directory.resolve(filename + ".jfr");
if (!Files.exists(p1)) {
return new DiskChunk(p1, nanos);
}
for (int i = 1; i < 100; i++) {
String s = Integer.toString(i);
if (i < 10) {
s = "0" + s;
}
Path p2 = directory.resolve(filename + "_" + s + ".jfr");
if (!Files.exists(p2)) {
return new DiskChunk(p2, nanos);
}
}
throw new IOException("Could not create chunk for path " + p1);
}
static String formatDateTime(LocalDateTime time) {
StringBuilder sb = new StringBuilder(19);
sb.append(time.getYear() / 100);
appendPadded(sb, time.getYear() % 100, true);
appendPadded(sb, time.getMonth().getValue(), true);
appendPadded(sb, time.getDayOfMonth(), true);
appendPadded(sb, time.getHour(), true);
appendPadded(sb, time.getMinute(), true);
appendPadded(sb, time.getSecond(), false);
return sb.toString();
}
private static void appendPadded(StringBuilder text, int number, boolean separator) {
if (number < 10) {
text.append('0');
}
text.append(number);
if (separator) {
text.append('_');
}
String filename = chunkFilename.next(d);
return new DiskChunk(Paths.get(filename), nanos);
}
@Override
@ -423,11 +391,11 @@ final class DiskRepository implements Closeable {
cleanUpDeadChunk(count + 10);
}
public synchronized void onChunkComplete(Instant timestamp) {
public synchronized void onChunkComplete(long endTimeNanos) {
int count = 0;
while (!activeChunks.isEmpty()) {
DiskChunk oldestChunk = activeChunks.peek();
if (oldestChunk.startTime.isBefore(timestamp)) {
if (oldestChunk.startTimeNanos < endTimeNanos) {
removeOldestChunk();
count++;
} else {

@ -29,13 +29,16 @@ import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
import jdk.jfr.internal.management.ManagementSupport;
final class DownLoadThread extends Thread {
private final RemoteRecordingStream stream;
private final Instant startTime;
private final Instant endTime;
private final DiskRepository diskRepository;
DownLoadThread(RemoteRecordingStream stream) {
DownLoadThread(RemoteRecordingStream stream, String name) {
super(name);
this.stream = stream;
this.startTime = stream.startTime;
this.endTime = stream.endTime;
@ -65,7 +68,7 @@ final class DownLoadThread extends Thread {
}
}
} catch (IOException ioe) {
// ignore
ManagementSupport.logDebug(ioe.getMessage());
} finally {
diskRepository.complete();
}

@ -133,8 +133,7 @@ public final class RemoteRecordingStream implements EventStream {
@Override
public void accept(Long endNanos) {
Instant t = ManagementSupport.epochNanosToInstant(endNanos);
repository.onChunkComplete(t);
repository.onChunkComplete(endNanos);
}
}
@ -552,8 +551,8 @@ public final class RemoteRecordingStream implements EventStream {
}
private void startDownload() {
Thread downLoadThread = new DownLoadThread(this);
downLoadThread.setName("JFR: Download Thread " + creationTime);
String name = "JFR: Download Thread " + creationTime;
Thread downLoadThread = new DownLoadThread(this, name);
downLoadThread.start();
}

@ -849,7 +849,6 @@ javax/script/Test7.java 8239361 generic-
jdk/jfr/event/runtime/TestNetworkUtilizationEvent.java 8228990,8229370 generic-all
jdk/jfr/event/compiler/TestCodeSweeper.java 8225209 generic-all
jdk/jfr/event/os/TestThreadContextSwitches.java 8247776 windows-all
jdk/jfr/jmx/streaming/TestRotate.java 8257215 generic-all
jdk/jfr/startupargs/TestStartName.java 8214685 windows-x64
jdk/jfr/startupargs/TestStartDuration.java 8214685 windows-x64

@ -44,7 +44,7 @@ import jdk.management.jfr.RemoteRecordingStream;
* @summary Tests that streaming can work over chunk rotations
* @requires vm.hasJFR
* @library /test/lib /test/jdk
* @run main/othervm jdk.jfr.jmx.streaming.TestRotate
* @run main/othervm -Xlog:jfr=debug jdk.jfr.jmx.streaming.TestRotate
*/
public class TestRotate {