From b83ae229260e8cdd9f1ebe2929e4ea31bd029362 Mon Sep 17 00:00:00 2001 From: Ahmed Alouane Date: Tue, 28 Mar 2023 15:41:47 +0100 Subject: [PATCH 1/6] that it? --- .../palantir/paxos/SqlitePaxosStateLog.java | 9 ++++--- .../management/PersistentNamespaceLoader.java | 1 + .../management/SqliteNamespaceLoader.java | 27 ++++++++++++++----- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java index f1f2c3baec8..54fdc04d81e 100644 --- a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java +++ b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java @@ -17,8 +17,8 @@ package com.palantir.paxos; import com.palantir.common.persist.Persistable; +import java.util.Optional; import java.util.OptionalLong; -import java.util.Set; import java.util.function.Function; import javax.sql.DataSource; import org.jdbi.v3.core.Jdbi; @@ -138,7 +138,10 @@ boolean[] writeBatchOfRounds( @Bind("useCase") String useCase, @BindPojo("round") Iterable> rounds); - @SqlQuery("SELECT DISTINCT(namespace) FROM paxosLog") - Set getAllNamespaces(); + @SqlQuery("SELECT MIN(namespace) FROM paxosLog") + Optional getSmallestNamespace(); + + @SqlQuery("SELECT MIN(namespace) FROM paxosLog WHERE namespace > :namespace") + Optional getNextSmallestNamespace(@Bind("namespace") String namespace); } } diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java index 3eaf6e22839..dea761525d7 100644 --- a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java +++ b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java @@ -22,6 +22,7 @@ public interface PersistentNamespaceLoader { /** * Gets all namespaces that have been persisted (via the persistence method under question). + * No transactionality guarantees are given. */ Set getAllPersistedNamespaces(); } diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java index c0efceacafe..5a9ad6a1581 100644 --- a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java +++ b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java @@ -18,8 +18,9 @@ import com.palantir.paxos.Client; import com.palantir.paxos.SqlitePaxosStateLog; +import java.util.HashSet; +import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import javax.sql.DataSource; import org.jdbi.v3.core.Jdbi; import org.jdbi.v3.sqlobject.SqlObjectPlugin; @@ -39,10 +40,24 @@ public static PersistentNamespaceLoader create(DataSource dataSource) { @Override public Set getAllPersistedNamespaces() { - return jdbi - .withExtension(SqlitePaxosStateLog.Queries.class, SqlitePaxosStateLog.Queries::getAllNamespaces) - .stream() - .map(Client::of) - .collect(Collectors.toSet()); + Set clients = new HashSet<>(); + + Optional currentNamespace = getSmallestNamespace(); + while (currentNamespace.isPresent()) { + String namespaceString = currentNamespace.get(); + clients.add(Client.of(namespaceString)); + currentNamespace = getNextSmallestNamespace(namespaceString); + } + + return clients; + } + + private Optional getSmallestNamespace() { + return jdbi.withExtension(SqlitePaxosStateLog.Queries.class, SqlitePaxosStateLog.Queries::getSmallestNamespace); + } + + private Optional getNextSmallestNamespace(String lastReadNamespace) { + return jdbi.withExtension( + SqlitePaxosStateLog.Queries.class, dao -> dao.getNextSmallestNamespace(lastReadNamespace)); } } From 561379dab3c389eca498ebc911081193b7e3fc78 Mon Sep 17 00:00:00 2001 From: Ahmed Alouane Date: Tue, 28 Mar 2023 15:54:33 +0100 Subject: [PATCH 2/6] variable naming --- .../src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java index 54fdc04d81e..753c689805a 100644 --- a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java +++ b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java @@ -142,6 +142,6 @@ boolean[] writeBatchOfRounds( Optional getSmallestNamespace(); @SqlQuery("SELECT MIN(namespace) FROM paxosLog WHERE namespace > :namespace") - Optional getNextSmallestNamespace(@Bind("namespace") String namespace); + Optional getNextSmallestNamespace(@Bind("namespace") String lastReadNamespace); } } From 7d84708b39f6c0759c09dd1e326322ce5c3f98e8 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Tue, 28 Mar 2023 14:58:05 +0000 Subject: [PATCH 3/6] Add generated changelog entries --- changelog/@unreleased/pr-6488.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-6488.v2.yml diff --git a/changelog/@unreleased/pr-6488.v2.yml b/changelog/@unreleased/pr-6488.v2.yml new file mode 100644 index 00000000000..d950412e346 --- /dev/null +++ b/changelog/@unreleased/pr-6488.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Improve Sqlite namespace loader implementation + links: + - https://github.com/palantir/atlasdb/pull/6488 From 56beb1bf9d0394d07288cb324b28314267fff5c1 Mon Sep 17 00:00:00 2001 From: Ahmed Alouane Date: Wed, 29 Mar 2023 12:26:10 +0100 Subject: [PATCH 4/6] api flag --- timelock-api/src/main/conjure/timelock-management-api.yml | 3 ++- .../atlasdb/timelock/management/PersistentNamespaceLoader.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/timelock-api/src/main/conjure/timelock-management-api.yml b/timelock-api/src/main/conjure/timelock-management-api.yml index 8fd2fe20d1f..81f2ab027cd 100644 --- a/timelock-api/src/main/conjure/timelock-management-api.yml +++ b/timelock-api/src/main/conjure/timelock-management-api.yml @@ -10,7 +10,8 @@ services: returns: set docs: | The endpoint loads all persisted namespaces. ``leaderPaxos`` is filtered out from the set - as it is not a namespace. + as it is not a namespace. No transactionality guarantees are given: namespace additions and + deletions while the request is running may or may not be reflected in the output. getActiveNamespaces: http: POST /getActiveNamespaces diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java index dea761525d7..f51d42df113 100644 --- a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java +++ b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/PersistentNamespaceLoader.java @@ -22,7 +22,8 @@ public interface PersistentNamespaceLoader { /** * Gets all namespaces that have been persisted (via the persistence method under question). - * No transactionality guarantees are given. + * No transactionality guarantees are given: namespace additions and deletions while the + * request is running may or may not be reflected in the output. */ Set getAllPersistedNamespaces(); } From c6888a628d89db6f83115d0be0a705f5745ed7ff Mon Sep 17 00:00:00 2001 From: aalouane <30903736+ergo14@users.noreply.github.com> Date: Wed, 29 Mar 2023 11:40:47 +0000 Subject: [PATCH 5/6] Autorelease 0.822.0-rc1 [skip ci] From 316464ca48d3f803e359bf24760fd9695cced55d Mon Sep 17 00:00:00 2001 From: Ahmed Alouane Date: Fri, 31 Mar 2023 02:31:43 +0100 Subject: [PATCH 6/6] sad --- .../com/palantir/paxos/SqlitePaxosStateLog.java | 9 ++++----- .../management/SqliteNamespaceLoader.java | 17 +++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java index 753c689805a..88bd214a582 100644 --- a/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java +++ b/leader-election-impl/src/main/java/com/palantir/paxos/SqlitePaxosStateLog.java @@ -138,10 +138,9 @@ boolean[] writeBatchOfRounds( @Bind("useCase") String useCase, @BindPojo("round") Iterable> rounds); - @SqlQuery("SELECT MIN(namespace) FROM paxosLog") - Optional getSmallestNamespace(); - - @SqlQuery("SELECT MIN(namespace) FROM paxosLog WHERE namespace > :namespace") - Optional getNextSmallestNamespace(@Bind("namespace") String lastReadNamespace); + // This is performant as long as the query plan is SEARCH. + @SqlQuery("SELECT MIN(namespace) FROM paxosLog WHERE namespace > :maybeLastReadNamespace") + Optional getNextLexicographicallySmallestNamespace( + @Bind("maybeLastReadNamespace") String maybeLastReadNamespace); } } diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java index 5a9ad6a1581..9fe3888160a 100644 --- a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java +++ b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/management/SqliteNamespaceLoader.java @@ -26,6 +26,8 @@ import org.jdbi.v3.sqlobject.SqlObjectPlugin; final class SqliteNamespaceLoader implements PersistentNamespaceLoader { + private static final String EMPTY_STRING = ""; + private final Jdbi jdbi; private SqliteNamespaceLoader(Jdbi jdbi) { @@ -42,22 +44,21 @@ public static PersistentNamespaceLoader create(DataSource dataSource) { public Set getAllPersistedNamespaces() { Set clients = new HashSet<>(); - Optional currentNamespace = getSmallestNamespace(); + // Namespaces contain at least one character implying the empty string is lexicographically strictly smaller + // than any namespace. + Optional currentNamespace = getNextLexicographicallySmallestNamespace(EMPTY_STRING); while (currentNamespace.isPresent()) { String namespaceString = currentNamespace.get(); clients.add(Client.of(namespaceString)); - currentNamespace = getNextSmallestNamespace(namespaceString); + currentNamespace = getNextLexicographicallySmallestNamespace(namespaceString); } return clients; } - private Optional getSmallestNamespace() { - return jdbi.withExtension(SqlitePaxosStateLog.Queries.class, SqlitePaxosStateLog.Queries::getSmallestNamespace); - } - - private Optional getNextSmallestNamespace(String lastReadNamespace) { + private Optional getNextLexicographicallySmallestNamespace(String maybeLastReadNamespace) { return jdbi.withExtension( - SqlitePaxosStateLog.Queries.class, dao -> dao.getNextSmallestNamespace(lastReadNamespace)); + SqlitePaxosStateLog.Queries.class, + dao -> dao.getNextLexicographicallySmallestNamespace(maybeLastReadNamespace)); } }