diff --git a/CHANGES.txt b/CHANGES.txt index f2d7040a1e..b3f3cf883a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.10 + * Improve 'Not enough space for compaction' logging messages (CASSANDRA-18260) * Incremental repairs fail on mixed IPv4/v6 addresses serializing SyncRequest (CASSANDRA-18474) * Deadlock updating sstable metadata if disk boundaries need reloading (CASSANDRA-18443) * Fix nested selection of reversed collections (CASSANDRA-17913) diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index cf770e6375..cecc60d75c 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -492,11 +492,32 @@ public class Directories continue; DataDirectoryCandidate candidate = new DataDirectoryCandidate(dataDir); // exclude directory if its total writeSize does not fit to data directory + logger.debug("DataDirectory {} has {} bytes available, checking if we can write {} bytes", dataDir.location, candidate.availableSpace, writeSize); if (candidate.availableSpace < writeSize) + { + logger.warn("DataDirectory {} can't be used for compaction. Only {} is available, but {} is the minimum write size.", + candidate.dataDirectory.location, + FileUtils.stringifyFileSize(candidate.availableSpace), + FileUtils.stringifyFileSize(writeSize)); continue; + } totalAvailable += candidate.availableSpace; } - return totalAvailable > expectedTotalWriteSize; + + if (totalAvailable <= expectedTotalWriteSize) + { + StringJoiner pathString = new StringJoiner(",", "[", "]"); + for (DataDirectory p: paths) + { + pathString.add(p.location.getAbsolutePath()); + } + logger.warn("Insufficient disk space for compaction. Across {} there's only {} available, but {} is needed.", + pathString.toString(), + FileUtils.stringifyFileSize(totalAvailable), + FileUtils.stringifyFileSize(expectedTotalWriteSize)); + return false; + } + return true; } public DataDirectory[] getWriteableLocations() diff --git a/test/unit/org/apache/cassandra/db/DirectoriesTest.java b/test/unit/org/apache/cassandra/db/DirectoriesTest.java index 2ba8952886..022c3a07d5 100644 --- a/test/unit/org/apache/cassandra/db/DirectoriesTest.java +++ b/test/unit/org/apache/cassandra/db/DirectoriesTest.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.Callable; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -30,11 +31,22 @@ import java.util.concurrent.Future; import com.google.common.collect.Sets; import org.apache.commons.lang3.StringUtils; +import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; +// Our version of Sfl4j seems to be missing the ListAppender class. +// Future sfl4j versions have one. At that time the below imports can be +// replaced with `org.slf4j.*` equivalents. +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; + import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.schema.Indexes; import org.apache.cassandra.schema.SchemaConstants; @@ -71,6 +83,13 @@ public class DirectoriesTest private static Set CFM; private static Map> files; + + private static final String MDCID = "test-DirectoriesTest-id"; + private static AtomicInteger diyThreadId = new AtomicInteger(1); + private int myDiyId = -1; + private static Logger logger; + private ListAppender listAppender; + @BeforeClass public static void beforeClass() { @@ -99,6 +118,7 @@ public class DirectoriesTest // Create two fake data dir for tests, one using CF directories, one that do not. createTestFiles(); + tailLogs(); } @AfterClass @@ -107,11 +127,27 @@ public class DirectoriesTest FileUtils.deleteRecursive(tempDataDir); } + @After + public void afterTest() + { + detachLogger(); + } + private static DataDirectory[] toDataDirectories(File location) { return new DataDirectory[] { new DataDirectory(location) }; } + private static DataDirectory[] toDataDirectories(File[] locations) + { + DataDirectory[] dirs = new DataDirectory[locations.length]; + for (int i=0; i(); + listAppender.start(); + + // add the appender to the logger + logger.addAppender(listAppender); + } + + private List filterLogByDiyId(List log) + { + ArrayList filteredLog = new ArrayList<>(); + for (ILoggingEvent event : log) + { + int mdcId = Integer.parseInt(event.getMDCPropertyMap().get(this.MDCID)); + if (mdcId == myDiyId) + { + filteredLog.add(event); + } + } + return filteredLog; + } + + private void checkFormattedMessage(List log, Level expectedLevel, String expectedMessage, int expectedCount) + { + int found=0; + for(ILoggingEvent e: log) + { + System.err.println(e.getFormattedMessage()); + if(e.getFormattedMessage().endsWith(expectedMessage)) + { + if (e.getLevel() == expectedLevel) + found++; + } + } + + assertEquals(expectedCount, found); + } + + @Test + public void testHasAvailableDiskSpace() + { + DataDirectory[] dataDirectories = new DataDirectory[] + { + new DataDirectory(new File("/nearlyFullDir1")) + { + public long getAvailableSpace() + { + return 11L; + } + }, + new DataDirectory(new File("/uniformDir2")) + { + public long getAvailableSpace() + { + return 999L; + } + }, + new DataDirectory(new File("/veryFullDir")) + { + public long getAvailableSpace() + { + return 4L; + } + } + }; + + Directories d = new Directories( ((TableMetadata) CFM.toArray()[0]), dataDirectories); + + assertTrue(d.hasAvailableDiskSpace(1,2)); + assertTrue(d.hasAvailableDiskSpace(10,99)); + assertFalse(d.hasAvailableDiskSpace(10,1024)); + assertFalse(d.hasAvailableDiskSpace(1024,1024*1024)); + + List filteredLog = listAppender.list; + //List filteredLog = filterLogByDiyId(listAppender.list); + // Log messages can be out of order, even for the single thread. (e tui AsyncAppender?) + // We can deal with it, it's sufficient to just check that all messages exist in the result + assertEquals(23, filteredLog.size()); + String logMsgFormat = "DataDirectory %s has %d bytes available, checking if we can write %d bytes"; + String logMsg = String.format(logMsgFormat, "/nearlyFullDir1", 11, 2); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/uniformDir2", 999, 2); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", 4, 2); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/nearlyFullDir1", 11, 2); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/uniformDir2", 999, 9); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", 4, 9); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/nearlyFullDir1", 11, 102); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/uniformDir2", 999, 102); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", 4, 102); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/nearlyFullDir1", 11, 1024); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/uniformDir2", 999, 1024); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", 4, 1024); + checkFormattedMessage(filteredLog, Level.DEBUG, logMsg, 1); + + logMsgFormat = "DataDirectory %s can't be used for compaction. Only %s is available, but %s is the minimum write size."; + logMsg = String.format(logMsgFormat, "/veryFullDir", "4 bytes", "9 bytes"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "/nearlyFullDir1", "11 bytes", "102 bytes"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", "4 bytes", "102 bytes"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "/nearlyFullDir1", "11 bytes", "1 KiB"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "/uniformDir2", "999 bytes", "1 KiB"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "/veryFullDir", "4 bytes", "1 KiB"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + + logMsgFormat = "Across %s there's only %s available, but %s is needed."; + logMsg = String.format(logMsgFormat, "[/nearlyFullDir1,/uniformDir2,/veryFullDir]", "999 bytes", "1 KiB"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + logMsg = String.format(logMsgFormat, "[/nearlyFullDir1,/uniformDir2,/veryFullDir]", "0 bytes", "1 MiB"); + checkFormattedMessage(filteredLog, Level.WARN, logMsg, 1); + } }