You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by mc...@apache.org on 2023/05/04 10:51:38 UTC
[cassandra] branch cassandra-4.0 updated: Improve 'Not enough space for compaction' logging messages
This is an automated email from the ASF dual-hosted git repository.
mck pushed a commit to branch cassandra-4.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.0 by this push:
new 65c99bfc42 Improve 'Not enough space for compaction' logging messages
65c99bfc42 is described below
commit 65c99bfc42015ccfbcbc8963b3fb3c2a761ea053
Author: Henrik Ingo <he...@avoinelama.fi>
AuthorDate: Wed Apr 19 17:34:23 2023 +0300
Improve 'Not enough space for compaction' logging messages
Adds separate WARN level messages to the check itself in Directories.
patch by Henrik Ingo; reviewed by Mick Semb Wever, Brad Schoening, Claude Warren, Kan Maung, maxwellguo for CASSANDRA-18260
---
CHANGES.txt | 1 +
src/java/org/apache/cassandra/db/Directories.java | 23 ++-
.../org/apache/cassandra/db/DirectoriesTest.java | 177 +++++++++++++++++++++
3 files changed, 200 insertions(+), 1 deletion(-)
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<TableMetadata> CFM;
private static Map<String, List<File>> 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<ILoggingEvent> 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<locations.length; i++)
+ {
+ dirs[i] = new DataDirectory(locations[i]);
+ }
+ return dirs;
+ }
+
private static void createTestFiles() throws IOException
{
for (TableMetadata cfm : CFM)
@@ -696,4 +732,145 @@ public class DirectoriesTest
return candidates;
}
+ private int getDiyThreadId()
+ {
+ return myDiyId = diyThreadId.getAndIncrement();
+ }
+
+ private void detachLogger()
+ {
+ logger.detachAppender(listAppender);
+ MDC.remove(this.MDCID);
+ }
+
+ private void tailLogs()
+ {
+ int diyId = getDiyThreadId();
+ MDC.put(this.MDCID, String.valueOf(diyId));
+ logger = (Logger) LoggerFactory.getLogger(Directories.class);
+
+ // create and start a ListAppender
+ listAppender = new ListAppender<>();
+ listAppender.start();
+
+ // add the appender to the logger
+ logger.addAppender(listAppender);
+ }
+
+ private List<ILoggingEvent> filterLogByDiyId(List<ILoggingEvent> log)
+ {
+ ArrayList<ILoggingEvent> 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<ILoggingEvent> 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<ILoggingEvent> filteredLog = listAppender.list;
+ //List<ILoggingEvent> 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);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org