henrikingo commented on code in PR #2284:
URL: https://github.com/apache/cassandra/pull/2284#discussion_r1172687643
##########
test/unit/org/apache/cassandra/db/DirectoriesTest.java:
##########
@@ -696,4 +732,144 @@ private List<Directories.DataDirectoryCandidate>
getWriteableDirectories(DataDir
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[]
Review Comment:
Yes. Thanks for saying this, I was hoping to discuss alternatives.
Your proposal will even make it easier to write and maintain these tests.
The reason I didn't do that already is that it's unclear to me how this will
affect the triaging of CI results. If you have a unit test like:
for ( value : hundredValues)
assertEquals(value, funtionToTest());
Now, if you get an assertion, how will you know which of the 100 values is
the wrong one? JUnite will just show you the line number of that line, but the
failure could be any of the 100 values we are looping over.
If you could suggest a way to do that and also maintain great usability for
those who run and ultimately have to fix it() anyway.
##########
src/java/org/apache/cassandra/db/Directories.java:
##########
@@ -485,18 +485,39 @@ public boolean hasAvailableDiskSpace(long
estimatedSSTables, long expectedTotalW
{
long writeSize = expectedTotalWriteSize / estimatedSSTables;
long totalAvailable = 0L;
+ boolean hasSpace = true;
for (DataDirectory dataDir : paths)
{
if
(DisallowedDirectories.isUnwritable(getLocationForDisk(dataDir)))
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);
Review Comment:
Actually, that message exists in trunk. I backported it in an effort to make
the very different <=4.1 branches at least a bit more similar to trunk. For
one thing, it allowed to reuse more test code.
We can remove it, but then I would ask, why wouldn't we remove it from trunk
as well. Where it exists already, before this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
