Simplified full disk check to just end execution if not --force is pa…

…ssed

If heap dump size cannot be determined, the execution not ends with an exception.

community/dbms/src/main/java/org/neo4j/commandline/dbms/DiagnosticsReportCommand.java

@@ -49,7 +49,7 @@
 import org.neo4j.diagnostics.DiagnosticsReportSource;
 import org.neo4j.diagnostics.DiagnosticsReportSources;
 import org.neo4j.diagnostics.DiagnosticsReporter;
-import org.neo4j.diagnostics.DiagnosticsReporterProgressInteractions;
+import org.neo4j.diagnostics.DiagnosticsReporterProgress;
 import org.neo4j.diagnostics.InteractiveProgress;
 import org.neo4j.diagnostics.NonInteractiveProgress;
 import org.neo4j.helpers.Args;
@@ -80,7 +80,6 @@ public class DiagnosticsReportCommand implements AdminCommand
     private final PrintStream out;
     private final FileSystemAbstraction fs;
     private final PrintStream err;
-    private boolean force;
 
     DiagnosticsReportCommand( Path homeDir, Path configDir, OutsideWorld outsideWorld )
     {
@@ -102,7 +101,7 @@ public void execute( String[] stringArgs ) throws IncorrectUsage, CommandFailed
         Args args = Args.withFlags( "list", "to", "verbose", "force" ).parse( stringArgs );
         verbose = args.has( "verbose" );
         jmxDumper = new JMXDumper( homeDir, fs, out, err, verbose );
-        force = args.has( "force" );
+        boolean force = args.has( "force" );
 
         DiagnosticsReporter reporter = createAndRegisterSources();
 
@@ -112,7 +111,7 @@ public void execute( String[] stringArgs ) throws IncorrectUsage, CommandFailed
             return;
         }
 
-        DiagnosticsReporterProgressInteractions progress = buildProgress();
+        DiagnosticsReporterProgress progress = buildProgress();
 
         // Start dumping
         Path destinationDir = new File( destinationArgument.parse( args ) ).toPath();
@@ -121,24 +120,24 @@ public void execute( String[] stringArgs ) throws IncorrectUsage, CommandFailed
             SimpleDateFormat dumpFormat = new SimpleDateFormat( "yyyy-MM-dd_HHmmss" );
             Path reportFile = destinationDir.resolve( dumpFormat.format( new Date() ) + ".zip" );
             out.println( "Writing report to " + reportFile.toAbsolutePath().toString() );
-            reporter.dump( classifiers.get(), reportFile, progress );
+            reporter.dump( classifiers.get(), reportFile, progress, force );
         }
         catch ( IOException e )
         {
             throw new CommandFailed( "Creating archive failed", e );
         }
     }
 
-    private DiagnosticsReporterProgressInteractions buildProgress()
+    private DiagnosticsReporterProgress buildProgress()
     {
-        DiagnosticsReporterProgressInteractions progress;
+        DiagnosticsReporterProgress progress;
         if ( System.console() != null )
         {
-            progress = new InteractiveProgress( out, verbose, force );
+            progress = new InteractiveProgress( out, verbose );
         }
         else
         {
-            progress = new NonInteractiveProgress( out, verbose, force );
+            progress = new NonInteractiveProgress( out, verbose );
         }
         return progress;
     }

community/dbms/src/main/java/org/neo4j/dbms/diagnostics/jmx/JmxDump.java

@@ -45,7 +45,7 @@
 
 import org.neo4j.diagnostics.DiagnosticsReportSource;
 import org.neo4j.diagnostics.DiagnosticsReportSources;
-import org.neo4j.diagnostics.DiagnosticsReporterProgressInteractions;
+import org.neo4j.diagnostics.DiagnosticsReporterProgress;
 import org.neo4j.diagnostics.ProgressAwareInputStream;
 
 /**
@@ -181,7 +181,7 @@ public String destinationPath()
             }
 
             @Override
-            public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressInteractions progress )
+            public void addToArchive( Path archiveDestination, DiagnosticsReporterProgress progress )
                     throws IOException
             {
                 // Heap dump has to target an actual file, we cannot stream directly to the archive
@@ -203,21 +203,13 @@ public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressIn
             }
 
             @Override
-            public long estimatedSize( DiagnosticsReporterProgressInteractions progress )
+            public long estimatedSize( DiagnosticsReporterProgress progress ) throws IOException
             {
-                try
-                {
-                    MemoryMXBean bean = ManagementFactory.getPlatformMXBean( mBeanServer, MemoryMXBean.class );
-                    long totalMemory = bean.getHeapMemoryUsage().getCommitted() + bean.getNonHeapMemoryUsage().getCommitted();
+                MemoryMXBean bean = ManagementFactory.getPlatformMXBean( mBeanServer, MemoryMXBean.class );
+                long totalMemory = bean.getHeapMemoryUsage().getCommitted() + bean.getNonHeapMemoryUsage().getCommitted();
 
-                    // We first write raw to disk then write to archive, 5x compression is a reasonable worst case estimation
-                    return (long) (totalMemory * 1.2);
-                }
-                catch ( IOException e )
-                {
-                    progress.error( "Unable to determine size of neo4j instance memory", e );
-                }
-                return 0;
+                // We first write raw to disk then write to archive, 5x compression is a reasonable worst case estimation
+                return (long) (totalMemory * 1.2);
             }
         };
     }
@@ -243,7 +235,7 @@ public String destinationPath()
             }
 
             @Override
-            public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressInteractions progress )
+            public void addToArchive( Path archiveDestination, DiagnosticsReporterProgress progress )
                     throws IOException
             {
                 try ( PrintStream printStream = new PrintStream( Files.newOutputStream( archiveDestination ) ) )
@@ -253,7 +245,7 @@ public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressIn
             }
 
             @Override
-            public long estimatedSize( DiagnosticsReporterProgressInteractions progress )
+            public long estimatedSize( DiagnosticsReporterProgress progress )
             {
                 return 0;
             }

community/kernel/src/main/java/org/neo4j/diagnostics/DiagnosticsReportSource.java

@@ -44,14 +44,15 @@ public interface DiagnosticsReportSource
      * @throws IOException if any file operations fail, exceptions should be handled by the caller for better error
      * reporting to the user.
      */
-    void addToArchive( Path archiveDestination, DiagnosticsReporterProgressInteractions progress ) throws IOException;
+    void addToArchive( Path archiveDestination, DiagnosticsReporterProgress progress ) throws IOException;
 
     /**
      * Returns an estimated upper bound of the input file size. Since the content will be placed in an archive the final
      * size can actually both increase and decrease.
      *
      * @param progress a monitor that can track progress.
      * @return the estimated file size in bytes.
+     * @throws IOException if size cannot be determined.
      */
-    long estimatedSize( DiagnosticsReporterProgressInteractions progress );
+    long estimatedSize( DiagnosticsReporterProgress progress ) throws IOException;
 }

community/kernel/src/main/java/org/neo4j/diagnostics/DiagnosticsReportSources.java

@@ -119,7 +119,7 @@ public String destinationPath()
         }
 
         @Override
-        public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressInteractions progress )
+        public void addToArchive( Path archiveDestination, DiagnosticsReporterProgress progress )
                 throws IOException
         {
             long size = fs.getFileSize( source );
@@ -133,7 +133,7 @@ public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressIn
         }
 
         @Override
-        public long estimatedSize( DiagnosticsReporterProgressInteractions progress )
+        public long estimatedSize( DiagnosticsReporterProgress progress )
         {
             return fs.getFileSize( source );
         }
@@ -157,7 +157,7 @@ public String destinationPath()
         }
 
         @Override
-        public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressInteractions progress )
+        public void addToArchive( Path archiveDestination, DiagnosticsReporterProgress progress )
                 throws IOException
         {
             String message = messageSupplier.get();
@@ -166,7 +166,7 @@ public void addToArchive( Path archiveDestination, DiagnosticsReporterProgressIn
         }
 
         @Override
-        public long estimatedSize( DiagnosticsReporterProgressInteractions progress )
+        public long estimatedSize( DiagnosticsReporterProgress progress )
         {
             return 0; // Size of strings should be negligible
         }

community/kernel/src/main/java/org/neo4j/diagnostics/DiagnosticsReporter.java

@@ -56,7 +56,7 @@ public void registerSource( String classifier, DiagnosticsReportSource source )
         additionalSources.computeIfAbsent( classifier, c -> new ArrayList<>() ).add( source );
     }
 
-    public void dump( Set<String> classifiers, Path destination, DiagnosticsReporterProgressInteractions progress ) throws IOException
+    public void dump( Set<String> classifiers, Path destination, DiagnosticsReporterProgress progress, boolean force ) throws IOException
     {
         // Collect sources
         List<DiagnosticsReportSource> sources = new ArrayList<>();
@@ -78,20 +78,8 @@ public void dump( Set<String> classifiers, Path destination, DiagnosticsReporter
         Path destinationFolder = destination.getParent();
         Files.createDirectories( destinationFolder );
 
-        // Estimate an upper bound of the final size and make sure it will fit
-        long estimatedFinalSize = sources.stream().mapToLong(
-                diagnosticsReportSource -> diagnosticsReportSource.estimatedSize( progress ) ).sum();
-        long freeSpace = destinationFolder.toFile().getFreeSpace();
-        if ( estimatedFinalSize > freeSpace )
-        {
-            String message =
-                    String.format( "WARNING: Free available disk space for %s is %s, worst case estimate is %s",
-                            destination.getFileName(), Format.bytes( freeSpace ), Format.bytes( estimatedFinalSize ) );
-            if ( !progress.shouldIgnorePotentialFullDisk( message ) )
-            {
-                return;
-            }
-        }
+        // Estimate an upper bound of the final size and make sure it will fit, if not, end reporting
+        estimateSizeAndCheckAvailableDiskSpace( destination, progress, sources, destinationFolder, force );
 
         // Compress all files to destination
         Map<String,String> env = new HashMap<>();
@@ -127,6 +115,31 @@ public void dump( Set<String> classifiers, Path destination, DiagnosticsReporter
         }
     }
 
+    private void estimateSizeAndCheckAvailableDiskSpace( Path destination,
+            DiagnosticsReporterProgress progress, List<DiagnosticsReportSource> sources,
+            Path destinationFolder, boolean force ) throws IOException
+    {
+        if ( force )
+        {
+            return;
+        }
+
+        long estimatedFinalSize = 0;
+        for ( DiagnosticsReportSource source  : sources )
+        {
+            estimatedFinalSize += source.estimatedSize( progress );
+        }
+
+        long freeSpace = destinationFolder.toFile().getFreeSpace();
+        if ( estimatedFinalSize > freeSpace )
+        {
+            String message = String.format(
+                    "Free available disk space for %s is %s, worst case estimate is %s. To ignore add '--force' to the command.",
+                    destination.getFileName(), Format.bytes( freeSpace ), Format.bytes( estimatedFinalSize ) );
+            throw new RuntimeException( message );
+        }
+    }
+
     public Set<String> getAvailableClassifiers()
     {
         return availableClassifiers;

community/kernel/src/main/java/org/neo4j/diagnostics/DiagnosticsReporterProgressInteractions.java → community/kernel/src/main/java/org/neo4j/diagnostics/DiagnosticsReporterProgress.java

@@ -23,7 +23,7 @@
  * Interface for handling feedback to the user. Implementations of this should be responsible of presenting the progress
  * to the user. Some specialised implementations can choose to omit any of the information provided here.
  */
-public interface DiagnosticsReporterProgressInteractions
+public interface DiagnosticsReporterProgress
 {
     /**
      * Calling this will notify the user that the percentage has changed.
@@ -48,14 +48,6 @@ public interface DiagnosticsReporterProgressInteractions
      */
     void error( String msg, Throwable throwable );
 
-    /**
-     * A check whether to ignore the free available disk space when generating the archive.
-     *
-     * @param message with information about the disk space.
-     * @return true if the execution should continue even if risking to fill up the disk.
-     */
-    boolean shouldIgnorePotentialFullDisk( String message );
-
     /**
      * @apiNote Called by dispatching class. Should not be called from diagnostics sources.
      */

community/kernel/src/main/java/org/neo4j/diagnostics/InteractiveProgress.java

@@ -26,22 +26,20 @@
  * Tracks progress in an interactive way, relies on the fact that the {@code PrintStream} echoes to a terminal that can
  * interpret the carrier return to reset the current line.
  */
-public class InteractiveProgress implements DiagnosticsReporterProgressInteractions
+public class InteractiveProgress implements DiagnosticsReporterProgress
 {
     private String prefix;
     private String suffix;
     private String totalSteps = "?";
     private final PrintStream out;
     private final boolean verbose;
-    private final boolean force;
     private String info = "";
     private int longestInfo;
 
-    public InteractiveProgress( PrintStream out, boolean verbose, boolean force )
+    public InteractiveProgress( PrintStream out, boolean verbose )
     {
         this.out = out;
         this.verbose = verbose;
-        this.force = force;
     }
 
     @Override
@@ -104,33 +102,6 @@ public void error( String msg, Throwable throwable )
         }
     }
 
-    @Override
-    public boolean shouldIgnorePotentialFullDisk( String message )
-    {
-        if ( force )
-        {
-            return true;
-        }
-        out.println( message );
-        while ( true )
-        {
-            out.print( "Ignore available disk space warning? [yN]: " );
-            String answer = System.console().readLine();
-            if ( answer.length() == 0 || answer.toLowerCase().equals( "n" ) )
-            {
-                return false;
-            }
-            else if ( answer.toLowerCase().equals( "y" ) )
-            {
-                return true;
-            }
-            else
-            {
-                out.println( "Unrecognised option: " + answer );
-            }
-        }
-    }
-
     @Override
     public void setTotalSteps( long steps )
     {

community/kernel/src/main/java/org/neo4j/diagnostics/NonInteractiveProgress.java

@@ -21,19 +21,17 @@
 
 import java.io.PrintStream;
 
-public class NonInteractiveProgress implements DiagnosticsReporterProgressInteractions
+public class NonInteractiveProgress implements DiagnosticsReporterProgress
 {
     private String totalSteps = "?";
     private final PrintStream out;
     private final boolean verbose;
-    private final boolean force;
     private int lastPercentage;
 
-    public NonInteractiveProgress( PrintStream out, boolean verbose, boolean force )
+    public NonInteractiveProgress( PrintStream out, boolean verbose )
     {
         this.out = out;
         this.verbose = verbose;
-        this.force = force;
     }
 
     @Override
@@ -82,18 +80,6 @@ public void error( String msg, Throwable throwable )
         }
     }
 
-    @Override
-    public boolean shouldIgnorePotentialFullDisk( String message )
-    {
-        if ( force )
-        {
-            return true;
-        }
-        out.println( message );
-        out.println( "To ignore available disk space warning, add '--force' to the command" );
-        return false;
-    }
-
     @Override
     public void setTotalSteps( long steps )
     {

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/OnDemandDetailsExecutionMonitor.java

@@ -43,7 +43,6 @@
 
 import static java.lang.Long.max;
 import static java.lang.System.currentTimeMillis;
-
 import static org.neo4j.helpers.Format.bytes;
 import static org.neo4j.helpers.Format.date;
 import static org.neo4j.helpers.Format.duration;
@@ -95,7 +94,7 @@ public OnDemandDetailsExecutionMonitor( PrintStream out, InputStream in, Monitor
     @Override
     public void initialize( DependencyResolver dependencyResolver )
     {
-        out.println( "Interactive command list (end with ENTER):" );
+        out.println( "InteractiveReporterInteractions command list (end with ENTER):" );
         actions.forEach( ( key, action ) -> out.println( "  " + key + ": " + action.first() ) );
         out.println();
         gcMonitor.start();