diff --git a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheck.java b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheck.java index aeac22b2f41..ec27a4cde56 100644 --- a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheck.java +++ b/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 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 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 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); } } diff --git a/atlasdb-dbkvs/src/test/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheckTest.java b/atlasdb-dbkvs/src/test/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheckTest.java index 69eabc02e09..7b2f2723c33 100644 --- a/atlasdb-dbkvs/src/test/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresVersionCheckTest.java +++ b/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); } } diff --git a/changelog/@unreleased/pr-7097.v2.yml b/changelog/@unreleased/pr-7097.v2.yml new file mode 100644 index 00000000000..de668b508ee --- /dev/null +++ b/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