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.

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

LOGMGR-139 Add maxAge to periodic-rotating-file-handlers #336

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class PeriodicRotatingFileHandler extends FileHandler {
private long nextRollover = Long.MAX_VALUE;
private TimeZone timeZone = TimeZone.getDefault();
private SuffixRotator suffixRotator = SuffixRotator.EMPTY;
private int maxAge = -1;
private String oldSuffix;

/**
* Construct a new instance with no formatter and no output file.
Expand Down Expand Up @@ -104,6 +106,19 @@ public PeriodicRotatingFileHandler(final File file, final String suffix, final b
setSuffix(suffix);
}

public PeriodicRotatingFileHandler(final File file, final String suffix, final int maxAge) throws FileNotFoundException {
super(file);
setSuffix(suffix);
setMaxAge(maxAge);
}

public PeriodicRotatingFileHandler(final File file, final String suffix, final int maxAge, final boolean append)
throws FileNotFoundException {
super(file, append);
setSuffix(suffix);
setMaxAge(maxAge);
}

@Override
public void setFile(final File file) throws FileNotFoundException {
synchronized (outputLock) {
Expand All @@ -119,10 +134,27 @@ protected void preWrite(final ExtLogRecord record) {
final long recordMillis = record.getMillis();
if (recordMillis >= nextRollover) {
rollOver();
if (maxAge > 0)
deleteOldFile(recordMillis);
calcNextRollover(recordMillis);
}
}

protected void deleteOldFile(final long fromTime) {
final File file = getFile();
if (file == null) {
// no file is set; a direct output stream or writer was specified
return;
}

final Calendar calendar = Calendar.getInstance(timeZone);
calendar.setTimeInMillis(fromTime);
calendar.add(Calendar.DAY_OF_MONTH, -maxAge);
oldSuffix = format.format(new Date(calendar.getTimeInMillis()));

suffixRotator.deleteFile(SecurityActions.getErrorManager(acc, this), file.toPath(), oldSuffix);
}

/**
* Set the suffix string. The string is in a format which can be understood by {@link SimpleDateFormat}.
* The period of the rotation is automatically calculated based on the suffix.
Expand Down Expand Up @@ -176,6 +208,12 @@ public void setSuffix(String suffix) throws IllegalArgumentException {
}
}

public void setMaxAge(int maxAge) {
synchronized (outputLock) {
this.maxAge = maxAge;
}
}

/**
* Returns the suffix to be used.
*
Expand All @@ -185,6 +223,10 @@ protected final String getNextSuffix() {
return nextSuffix;
}

protected final String getOldSuffix() {
return oldSuffix;
}

/**
* Returns the file rotator for this handler.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,29 @@ public PeriodicSizeRotatingFileHandler(final File file, final String suffix, fin
this.maxBackupIndex = maxBackupIndex;
}

public PeriodicSizeRotatingFileHandler(final File file, final String suffix, final int maxAge)
throws FileNotFoundException {
super(file, suffix, maxAge);
}

public PeriodicSizeRotatingFileHandler(final File file, final String suffix, final int maxAge, final boolean append)
throws FileNotFoundException {
super(file, suffix, maxAge, append);
}

public PeriodicSizeRotatingFileHandler(final File file, final String suffix, final long rotateSize,
final int maxBackupIndex, final int maxAge) throws FileNotFoundException {
super(file, suffix, maxAge);
this.rotateSize = rotateSize;
this.maxBackupIndex = maxBackupIndex;
}

public PeriodicSizeRotatingFileHandler(final File file, final String suffix, final long rotateSize,
final int maxBackupIndex, final int maxAge, final boolean append) throws FileNotFoundException {
super(file, suffix, maxAge, append);
this.rotateSize = rotateSize;
this.maxBackupIndex = maxBackupIndex;
}

@Override
public void setOutputStream(final OutputStream outputStream) {
Expand Down Expand Up @@ -240,6 +263,21 @@ protected void preWrite(final ExtLogRecord record) {
}
}

@Override
protected void deleteOldFile(long fromTime) {
super.deleteOldFile(fromTime);
final File file = getFile();
if (file == null) {
// no file is set; a direct output stream or writer was specified
return;
}
final int maxBackupIndex = this.maxBackupIndex;

for (int i = 1; i <= maxBackupIndex; i++) {
getSuffixRotator().deleteFile(SecurityActions.getErrorManager(acc, this), file.toPath(), getOldSuffix() + "." + i);
}
}

private void setFileInternal(final File file, final boolean doPrivileged) throws FileNotFoundException {
if (System.getSecurityManager() == null || !doPrivileged) {
super.setFile(file);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,15 @@ void rotate(final ErrorManager errorManager, final Path source, final String suf
}
}

void deleteFile(final ErrorManager errorManager, final Path source, final String suffix) {
final Path target = Paths.get(source + suffix + compressionSuffix);
try {
deleteFile(target);
} catch (Exception e) {
errorManager.error(String.format("Failed to delete file %s", target), e, ErrorManager.GENERIC_FAILURE);
}
}

@Override
public String toString() {
return originalSuffix;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class PeriodicRotatingFileHandlerTests extends AbstractHandlerTest {
@Before
public void createHandler() throws FileNotFoundException {
// Create the handler
handler = new PeriodicRotatingFileHandler(logFile.toFile(), rotateFormatter.toPattern(), false);
handler = new PeriodicRotatingFileHandler(logFile.toFile(), rotateFormatter.toPattern(), 3, false);
handler.setFormatter(FORMATTER);
// Set append to true to ensure the rotated file is overwritten
handler.setAppend(true);
Expand Down Expand Up @@ -149,8 +149,6 @@ record = createLogRecord(Level.INFO, "Date: %s", nextDate);

private void testRotate(final Calendar cal, final Path rotatedFile) throws Exception {
final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
final int currentDay = cal.get(Calendar.DAY_OF_MONTH);
final int nextDay = currentDay + 1;

final String currentDate = sdf.format(cal.getTime());

Expand All @@ -166,7 +164,7 @@ private void testRotate(final Calendar cal, final Path rotatedFile) throws Excep
Assert.assertTrue("Expected the line to contain the date: " + currentDate, lines.get(0).contains(currentDate));

// Create a new record, increment the day by one and validate
cal.add(Calendar.DAY_OF_MONTH, nextDay);
cal.add(Calendar.DAY_OF_MONTH, 1);
final String nextDate = sdf.format(cal.getTime());
record = createLogRecord(Level.INFO, "Date: %s", nextDate);
record.setMillis(cal.getTimeInMillis());
Expand All @@ -182,6 +180,20 @@ record = createLogRecord(Level.INFO, "Date: %s", nextDate);
lines = Files.readAllLines(rotatedFile, StandardCharsets.UTF_8);
Assert.assertEquals("More than 1 line found", 1, lines.size());
Assert.assertTrue("Expected the line to contain the date: " + currentDate, lines.get(0).contains(currentDate));

cal.add(Calendar.DAY_OF_MONTH, 1);
record.setMillis(cal.getTimeInMillis());
handler.publish(record);
cal.add(Calendar.DAY_OF_MONTH, 1);
record.setMillis(cal.getTimeInMillis());
handler.publish(record);

cal.add(Calendar.DAY_OF_MONTH, -3);
Assert.assertFalse(Files.exists(BASE_LOG_DIR.toPath().resolve(FILENAME + rotateFormatter.format(cal.getTime()))));
cal.add(Calendar.DAY_OF_MONTH, 1);
Assert.assertTrue(Files.exists(BASE_LOG_DIR.toPath().resolve(FILENAME + rotateFormatter.format(cal.getTime()))));
cal.add(Calendar.DAY_OF_MONTH, 1);
Assert.assertTrue(Files.exists(BASE_LOG_DIR.toPath().resolve(FILENAME + rotateFormatter.format(cal.getTime()))));
}

private void testArchiveRotate(final String archiveSuffix) throws Exception {
Expand Down Expand Up @@ -209,6 +221,7 @@ record = createLogRecord(Level.INFO, "Date: %s", nextDate);

// Create a new record, increment the day by one and validate
date = date.plusDays(1);
final String thirdDateSuffix = rotateFormatter.format(date);
final String thirdDay = formatter.format(date);
record = createLogRecord(Level.INFO, "Date: %s", thirdDay);
record.setMillis(date.toInstant().toEpochMilli());
Expand All @@ -233,5 +246,18 @@ record = createLogRecord(Level.INFO, "Date: %s", thirdDay);
Assert.fail("Unknown archive suffix: " + archiveSuffix);
}
compareArchiveContents(rotated1, rotated2, logFile.getFileName().toString());

date = date.plusDays(1);
final String fourthDay = formatter.format(date);
record = createLogRecord(Level.INFO, "Date: %s", fourthDay);
record.setMillis(date.toInstant().toEpochMilli());
handler.publish(record);

// There are three files, not four files.
final Path rotated3 = logDir.resolve(FILENAME + thirdDateSuffix + archiveSuffix);
Assert.assertTrue(Files.exists(logFile));
Assert.assertFalse(Files.exists(rotated1));
Assert.assertTrue(Files.exists(rotated2));
Assert.assertTrue(Files.exists(rotated3));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.file.Path;
import java.text.SimpleDateFormat;
import java.time.LocalDate;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Calendar;
Expand Down Expand Up @@ -252,14 +253,13 @@ private void testArchiveRotate(final String dateSuffix, final String archiveSuff
handler.setSuffix((dateSuffix == null ? "" : dateSuffix) + archiveSuffix);
// Set append to true to ensure the rotated file is overwritten
handler.setAppend(true);
handler.setMaxAge(3);

// Allow a few rotates
for (int i = 0; i < 100; i++) {
handler.publish(createLogRecord("Test message: %d", i));
}

handler.close();

// We should end up with 3 files, 2 rotated and the default log
final Path logDir = BASE_LOG_DIR.toPath();
final Path path1 = logDir.resolve(FILENAME + currentDate + ".1" + archiveSuffix);
Expand All @@ -281,9 +281,38 @@ private void testArchiveRotate(final String dateSuffix, final String archiveSuff

compareArchiveContents(path1, path2, logFile.getName());

// Clean up files
Files.deleteIfExists(path1);
Files.deleteIfExists(path2);
ZonedDateTime date = ZonedDateTime.now();
ExtLogRecord record = createLogRecord("Test message!");
for (int j = 0; j < 3; j++) {
date = date.plusDays(1);
record.setMillis(date.toInstant().toEpochMilli());
for (int i = 0; i < 100; i++) {
handler.publish(record);
}
}

if (dateSuffix != null) {
Assert.assertFalse(Files.exists(path1));
Assert.assertFalse(Files.exists(path2));
Assert.assertTrue(Files.exists(
logDir.resolve(FILENAME + date.minusDays(2).format(DateTimeFormatter.ofPattern(dateSuffix)) + archiveSuffix)));
Assert.assertTrue(Files.exists(logDir
.resolve(FILENAME + date.minusDays(2).format(DateTimeFormatter.ofPattern(dateSuffix)) + ".1" + archiveSuffix)));
Assert.assertTrue(Files.exists(logDir
.resolve(FILENAME + date.minusDays(2).format(DateTimeFormatter.ofPattern(dateSuffix)) + ".2" + archiveSuffix)));
Assert.assertTrue(Files.exists(
logDir.resolve(FILENAME + date.minusDays(1).format(DateTimeFormatter.ofPattern(dateSuffix)) + archiveSuffix)));
Assert.assertTrue(Files.exists(logDir
.resolve(FILENAME + date.minusDays(1).format(DateTimeFormatter.ofPattern(dateSuffix)) + ".1" + archiveSuffix)));
Assert.assertTrue(Files.exists(logDir
.resolve(FILENAME + date.minusDays(1).format(DateTimeFormatter.ofPattern(dateSuffix)) + ".2" + archiveSuffix)));
Assert.assertTrue(Files.exists(
logDir.resolve(FILENAME + date.format(DateTimeFormatter.ofPattern(dateSuffix)) + ".1" + archiveSuffix)));
Assert.assertTrue(Files.exists(
logDir.resolve(FILENAME + date.format(DateTimeFormatter.ofPattern(dateSuffix)) + ".2" + archiveSuffix)));
}
handler.close();

}

private void testPeriodicAndSizeRotate0(int handlerPeriod, int logMessagePeriod, boolean testSize) throws Exception {
Expand Down