From 681740b370971ed4936ac002fc288e63a071f852 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sun, 3 Nov 2019 23:56:28 +0100 Subject: Use faster and more approximate snippetting --- .../task/util/process/ChildProcessException.java | 54 ++++----------- .../admin/task/util/text/SnippetGenerator.java | 76 +++++----------------- .../admin/task/util/text/SnippetGeneratorTest.java | 39 ++++------- 3 files changed, 41 insertions(+), 128 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java index cbc8ffbf1b7..bc88702b0fc 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process; +import com.yahoo.vespa.hosted.node.admin.task.util.text.SnippetGenerator; + /** * Base class for child process related exceptions, with a util to build an error message * that includes a large part of the output. @@ -10,10 +12,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process; */ @SuppressWarnings("serial") public abstract class ChildProcessException extends RuntimeException { - private static final int MAX_OUTPUT_PREFIX = 200; - private static final int MAX_OUTPUT_SUFFIX = 200; - // Omitting a number of chars less than 10 or less than 10% would be ridiculous. - private static final int MAX_OUTPUT_SLACK = Math.max(10, (10 * (MAX_OUTPUT_PREFIX + MAX_OUTPUT_SUFFIX)) / 100); + private static final SnippetGenerator snippetGenerator = new SnippetGenerator(); /** * An exception with a message of the following format: @@ -36,44 +35,13 @@ public abstract class ChildProcessException extends RuntimeException { super(makeSnippet(problem, commandLine, possiblyHugeOutput), cause); } - private static String makeSnippet(String problem, - String commandLine, - String possiblyHugeOutput) { - return makeSnippet( - problem, - commandLine, - possiblyHugeOutput, - MAX_OUTPUT_PREFIX, - MAX_OUTPUT_SUFFIX, - MAX_OUTPUT_SLACK); - } - - // Package-private instead of private for testing. - static String makeSnippet(String problem, - String commandLine, - String possiblyHugeOutput, - int maxOutputPrefix, - int maxOutputSuffix, - int maxOutputSlack) { - StringBuilder stringBuilder = new StringBuilder() - .append("Command '") - .append(commandLine) - .append("' ") - .append(problem) - .append(": stdout/stderr: '"); - - if (possiblyHugeOutput.length() <= maxOutputPrefix + maxOutputSuffix + maxOutputSlack) { - stringBuilder.append(possiblyHugeOutput); - } else { - stringBuilder.append(possiblyHugeOutput, 0, maxOutputPrefix) - .append("... [") - .append(possiblyHugeOutput.length() - maxOutputPrefix - maxOutputSuffix) - .append(" chars omitted] ...") - .append(possiblyHugeOutput.substring(possiblyHugeOutput.length() - maxOutputSuffix)); - } - - stringBuilder.append("'"); - - return stringBuilder.toString(); + private static String makeSnippet(String problem, String commandLine, String possiblyHugeOutput) { + return "Command '" + + commandLine + + "' " + + problem + + ": stdout/stderr: '" + + snippetGenerator.makeSnippet(possiblyHugeOutput, 500) + + "'"; } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java index 1cb75434486..9a1876eb523 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java @@ -2,73 +2,29 @@ package com.yahoo.vespa.hosted.node.admin.task.util.text; /** - * @author hakonhall + * @author hakon */ public class SnippetGenerator { - public static final String OMIT_PREFIX = "[..."; - public static final String OMIT_SUFFIX = " chars omitted]"; - /** - * If {@code maxLength in }{@link #makeSnippet(String, int maxLength)} is less than this limit, - * a snippet would actually be larger than maxLength. - */ - public static final int LEAST_MAXLENGTH = OMIT_PREFIX.length() + 2 + OMIT_SUFFIX.length(); + private static final int ASSUMED_OMIT_TEXT_LENGTH = omitText(1234).length(); - public String makeSnippet(String possiblyHugeString, int maxLength) { - if (possiblyHugeString.length() <= maxLength) { - return possiblyHugeString; - } + private static String omitText(int omittedLength) { return "[..." + omittedLength + " chars omitted]"; } - // We will preserve a prefix and suffix, but replace the middle part of possiblyHugeString - // with a cut text: - // possiblyHugeString = prefix + omitted + suffix - // result = prefix + cut + suffix - // cut = OMIT_PREFIX + size + OMIT_SUFFIX - // size = format("%d", omitted.length()) - // digits = size.length() - int assumedDigits = 2; // Just an initial guess: size.length() between 10 and 99 - int prefixLength, suffixLength; - String size; - - while (true) { - int assumedCutLength = OMIT_PREFIX.length() + assumedDigits + OMIT_SUFFIX.length(); - // Make prefixLength ~ suffixLength, with prefixLength >= suffixLength - suffixLength = Math.max((maxLength - assumedCutLength) / 2, 0); - prefixLength = Math.max(maxLength - assumedCutLength - suffixLength, 0); - // RHS is guaranteed to be >= 0 - int omittedLength = possiblyHugeString.length() - prefixLength - suffixLength; - size = String.format("%d", omittedLength); - - // If assumedCutLength happens to be wrong, we retry with an adjusted setting. - int actualDigits = size.length(); - if (actualDigits == assumedDigits) { - break; - } - - // Is this loop guaranteed to finish? Yes, because from one iteration to the next, - // omittedLength can change by at most 9 (size having 1 digit to size with 10 digits - // or vice versa): - // - If actualDigits < assumedDigits, omittedLength will decrease on next iteration - // by 1-9, and so size can at most decrease by another 1 for that iteration. And, - // if it did decrease by 1, it cannot decrease again (and must therefore break - // the loop) in the iteration after, since a drop of (at most) 18 cannot remove - // 2 digits from a number. - // - If actualDigits > assumedDigits, a similar argument holds. - assumedDigits = actualDigits; - } + /** Returns a snippet of approximate size. */ + public String makeSnippet(String text, int sizeHint) { + if (text.length() < sizeHint) return text; + int maxSuffixLength = Math.max(0, (sizeHint - ASSUMED_OMIT_TEXT_LENGTH) / 2); + int maxPrefixLength = Math.max(0, sizeHint - ASSUMED_OMIT_TEXT_LENGTH - maxSuffixLength); String snippet = - possiblyHugeString.substring(0, prefixLength) + - OMIT_PREFIX + - size + - OMIT_SUFFIX + - possiblyHugeString.substring(possiblyHugeString.length() - suffixLength); + text.substring(0, maxPrefixLength) + + omitText(text.length() - maxPrefixLength - maxSuffixLength) + + text.substring(text.length() - maxSuffixLength); + + // It would be silly to return a snippet when the full text is barely longer. + // Note: Say ASSUMED_OMIT_TEXT_LENGTH=23: text will be returned whenever sizeHint<23 and text.length()<28. + if (text.length() <= 1.05 * snippet.length() + 5) return text; - if (snippet.length() > maxLength) { - // This can happen if maxLength is too small. - return possiblyHugeString.substring(0, maxLength); - } else { - return snippet; - } + return snippet; } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java index 59ae4633dce..8a67052a286 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java @@ -6,61 +6,50 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; /** - * @author hakonhall + * @author hakon */ public class SnippetGeneratorTest { private final SnippetGenerator generator = new SnippetGenerator(); - private void assertSnippet(String text, int maxLength, String expectedSnippet) { - assertEquals(expectedSnippet, generator.makeSnippet(text, maxLength)); + private void assertSnippet(String text, int sizeHint, String expectedSnippet) { + assertEquals(expectedSnippet, generator.makeSnippet(text, sizeHint)); } @Test - public void prefixSnippetForReallySmallMaxLength() { + public void prefixSnippetForReallySmallSizeHint() { assertSnippet( "This is a long text that should be snippeted", 0, - ""); + "[...44 chars omitted]"); assertSnippet( "This is a long text that should be snippeted", 1, - "T"); - - assertSnippet( - "This is a long text that should be snippeted", 10, - "This is a "); - - assertSnippet( - "This is a long text that should be snippeted", 20, - "This is a long text "); + "[...44 chars omitted]"); } @Test public void snippet() { assertSnippet( - "This is a long text that should be snippeted", 21, + "This is a long text that should be snippeted", 23, "[...44 chars omitted]"); assertSnippet( - "This is a long text that should be snippeted", 22, + "This is a long text that should be snippeted", 24, "T[...43 chars omitted]"); assertSnippet( "This is a long text that should be snippeted", 30, - "This [...35 chars omitted]eted"); + "This[...37 chars omitted]ted"); - assertSnippet( - "This is a long text that should be snippeted", 31, - "This [...34 chars omitted]peted"); - - assertSnippet( - "This is a long text that should be snippeted", 43, - "This is a l[...22 chars omitted]e snippeted"); } @Test public void noShorteningNeeded() { assertSnippet( - "This is a long text that should be snippeted", 44, + "This is a long text that should be snippeted", 39, + "This is [...28 chars omitted]nippeted"); + + assertSnippet( + "This is a long text that should be snippeted", 40, "This is a long text that should be snippeted"); assertSnippet( -- cgit v1.2.3