Update postgres version checking to handle verbose version strings

atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheck.java

@@ -15,34 +15,71 @@
  */
 package com.palantir.atlasdb.keyvalue.dbkvs.impl.postgres;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Suppliers;
+import com.palantir.logsafe.SafeArg;
 import com.palantir.util.AssertUtils;
 import com.palantir.util.VersionStrings;
+import java.util.Optional;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import org.slf4j.Logger;
 
 public final class PostgresVersionCheck {
     static final String MIN_POSTGRES_VERSION = "9.6";
+    private static final Supplier<Pattern> VERSION_PATTERN = Suppliers.memoize(() -> {
+        // Matches: x, x.y, x.y.z, etc in group 1
+        // Allows discarding an optional additional section in parens, for example:
+        // 14.11 (Ubuntu 14.11-1.pgdg20.04+1)
+        return Pattern.compile("^([\\.0-9]+)( \\([^\\)]+\\))?$");
+    });
 
     private PostgresVersionCheck() {}
 
+    @VisibleForTesting
+    static Optional<String> extractValidPostgresVersion(String rawVersionString) {
+        Matcher matcher = VERSION_PATTERN.get().matcher(rawVersionString);
+        if (matcher.matches()) {
+            return Optional.of(matcher.group(1));
+        } else {
+            return Optional.empty();
+        }
+    }
+
     public static void checkDatabaseVersion(String version, Logger log) {
-        boolean checkPasses =
-                version.matches("^[\\.0-9]+$") && VersionStrings.compareVersions(version, MIN_POSTGRES_VERSION) >= 0;
-        if (VersionStrings.compareVersions(version, "9.5") >= 0
-                && VersionStrings.compareVersions(version, "9.5.2") < 0) {
-            // N.B. This situation is bad. Do not just log a warning and assert - actually throw an error.
-            throw new DbkvsVersionException(
-                    "You are running Postgres " + version + ". Versions 9.5.0 and 9.5.1 contain a known bug "
-                            + "that causes incorrect results to be returned for certain queries. "
-                            + "Please update your Postgres distribution.");
+        Optional<String> parsedVersion = extractValidPostgresVersion(version);
+        if (parsedVersion.isPresent()) {
+            log.info("Parsed Postgres version", SafeArg.of("parsed", parsedVersion.get()), SafeArg.of("raw", version));
+            boolean greaterThanMinVersion =
+                    VersionStrings.compareVersions(parsedVersion.get(), MIN_POSTGRES_VERSION) >= 0;
+            if (VersionStrings.compareVersions(parsedVersion.get(), "9.5") >= 0
+                    && VersionStrings.compareVersions(parsedVersion.get(), "9.5.2") < 0) {
+                // N.B. This situation is bad. Do not just log a warning and assert - actually throw an error.
+                throw new DbkvsVersionException(
+                        "You are running Postgres " + version + ". Versions 9.5.0 and 9.5.1 contain a known bug "
+                                + "that causes incorrect results to be returned for certain queries. "
+                                + "Please update your Postgres distribution.");
+            }
+            AssertUtils.assertAndLog(
+                    log,
+                    greaterThanMinVersion,
+                    "Your key value service currently uses version %s of postgres, parsed to %s."
+                            + " The minimum supported version is %s."
+                            + " If you absolutely need to use an older version of postgres,"
+                            + " please contact Palantir support for assistance.",
+                    version,
+                    parsedVersion,
+                    MIN_POSTGRES_VERSION);
+        } else {
+            AssertUtils.assertAndLog(
+                    log,
+                    false,
+                    "Unable to parse a version from postgres."
+                            + " Raw format string was %s. Please contact Palantir support for assistance."
+                            + " Minimum allowed Postgres version is %s.",
+                    version,
+                    MIN_POSTGRES_VERSION);
         }
-        AssertUtils.assertAndLog(
-                log,
-                checkPasses,
-                "Your key value service currently uses version %s of postgres."
-                        + " The minimum supported version is %s."
-                        + " If you absolutely need to use an older version of postgres,"
-                        + " please contact Palantir support for assistance.",
-                version,
-                MIN_POSTGRES_VERSION);
     }
 }

atlasdb-dbkvs/src/test/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheckTest.java

@@ -32,25 +32,63 @@ public class PostgresVersionCheckTest {
     @Test
     @SuppressWarnings(value = "Slf4jConstantLogMessage")
     public void shouldLogErrorOn_9_2_24() {
-        verifyLowVersionLogsError("9.2.24");
+        verifyLowVersionLogsError("9.2.24", "9.2.24");
     }
 
     @Test
     public void shouldLogErrorOn_9_5_2() {
-        verifyLowVersionLogsError("9.5.2");
+        verifyLowVersionLogsError("9.5.2", "9.5.2");
+    }
+
+    @Test
+    public void shouldLogErrorOn_9_5_2_verbose() {
+        verifyLowVersionLogsError("9.5.2 (Ubuntu 9.5.2.pgdg20.04+1)", "9.5.2");
+    }
+
+    @Test
+    public void shouldLogErrorOnUnparseable() {
+        verifyUnparseableVersionError("123A");
+    }
+
+    @Test
+    public void shouldLogErrorOnEmpty() {
+        verifyUnparseableVersionError("");
     }
 
     @SuppressWarnings("Slf4jConstantLogMessage")
-    private static void verifyLowVersionLogsError(String lowVersion) {
+    private static void verifyUnparseableVersionError(String version) {
+        Logger log = mock(Logger.class);
+        String expectedMessage = "Unable to parse a version from postgres";
+        assertThatThrownBy(() -> PostgresVersionCheck.checkDatabaseVersion(version, log))
+                .isInstanceOf(AssertionError.class)
+                .hasMessageContaining(expectedMessage);
+        verify(log)
+                .error(
+                        eq("Assertion with exception!"),
+                        eq(version),
+                        eq(PostgresVersionCheck.MIN_POSTGRES_VERSION),
+                        isA(SafeArg.class),
+                        Mockito.any(Exception.class));
+        verifyNoMoreInteractions(log);
+    }
+
+    @SuppressWarnings("Slf4jConstantLogMessage")
+    private static void verifyLowVersionLogsError(String lowVersion, String parsed) {
         Logger log = mock(Logger.class);
         String expectedMessage = "The minimum supported version is";
         assertThatThrownBy(() -> PostgresVersionCheck.checkDatabaseVersion(lowVersion, log))
                 .isInstanceOf(AssertionError.class)
                 .hasMessageContaining(expectedMessage);
+        verify(log)
+                .info(
+                        eq("Parsed Postgres version"),
+                        eq(SafeArg.of("parsed", parsed)),
+                        eq(SafeArg.of("raw", lowVersion)));
         verify(log)
                 .error(
                         eq("Assertion with exception!"),
                         eq(lowVersion),
+                        eq(PostgresVersionCheck.extractValidPostgresVersion(lowVersion)),
                         eq(PostgresVersionCheck.MIN_POSTGRES_VERSION),
                         isA(SafeArg.class),
                         Mockito.any(Exception.class));
@@ -74,6 +112,24 @@ public void shouldFailOn_9_5_1() {
     public void shouldBeFineOn_9_6_12() {
         Logger log = mock(Logger.class);
         PostgresVersionCheck.checkDatabaseVersion("9.6.12", log);
+        verify(log)
+                .info(
+                        eq("Parsed Postgres version"),
+                        eq(SafeArg.of("parsed", "9.6.12")),
+                        eq(SafeArg.of("raw", "9.6.12")));
+        verifyNoMoreInteractions(log);
+    }
+
+    @Test
+    @SuppressWarnings("Slf4jConstantLogMessage")
+    public void shouldBeFineOn_14_11_verbose() {
+        Logger log = mock(Logger.class);
+        PostgresVersionCheck.checkDatabaseVersion("14.11 (Ubuntu 14.11-1.pgdg20.04+1)", log);
+        verify(log)
+                .info(
+                        eq("Parsed Postgres version"),
+                        eq(SafeArg.of("parsed", "14.11")),
+                        eq(SafeArg.of("raw", "14.11 (Ubuntu 14.11-1.pgdg20.04+1)")));
         verifyNoMoreInteractions(log);
     }
 }

changelog/@unreleased/pr-7097.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Update postgres version checking to handle verbose version strings
+  links:
+  - https://github.com/palantir/atlasdb/pull/7097