Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Filter out leaderPaxos from list of namespaces #4760

Merged
merged 11 commits into from
May 12, 2020
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-4760.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: fix
fix:
description: '`leaderPaxos` will not be returned as a namespace. Previously, `leaderPaxos`
was incorrectly considered as a namespace.'
links:
- https://github.com/palantir/atlasdb/pull/4760
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.palantir.atlasdb.timelock.paxos.PaxosTimeLockConstants;
import com.palantir.atlasdb.timelock.paxos.PaxosUseCase;
import com.palantir.logsafe.SafeArg;
import com.palantir.paxos.Client;
Expand All @@ -44,6 +45,7 @@ public Set<Client> getAllPersistedNamespaces() {
.filter(useCase -> useCase != PaxosUseCase.LEADER_FOR_ALL_CLIENTS)
.map(useCase -> useCase.logDirectoryRelativeToDataDirectory(rootDataDirectory))
.flatMap(DiskNamespaceLoader::getNamespacesFromUseCaseResolvedDirectory)
.filter(namespace -> !namespace.equals(PaxosTimeLockConstants.LEADER_PAXOS_NAMESPACE))
.map(Client::of)
.collect(Collectors.toSet());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.atlasdb.timelock.management;

import static org.assertj.core.api.Assertions.assertThat;

import java.nio.file.Paths;
import java.util.Set;
import java.util.stream.Collectors;

import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import com.palantir.atlasdb.timelock.paxos.PaxosTimeLockConstants;

public class DiskNamespaceLoaderTest {
private static final String NAMESPACE_1 = "namespace_1";
private static final String NAMESPACE_2 = "namespace_2";


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: 2 lines?

public final @Rule TemporaryFolder tempFolder = new TemporaryFolder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public final @Rule TemporaryFolder tempFolder = new TemporaryFolder();
@Rule
public final TemporaryFolder tempFolder = new TemporaryFolder();

This is fine also, just for consistency with the other tests.


public DiskNamespaceLoader diskNamespaceLoader;

@Before
public void setup() {
diskNamespaceLoader = new DiskNamespaceLoader(tempFolder.getRoot().toPath());
createDirectoryForLeaderForEachClientUseCase(NAMESPACE_1);
createDirectoryInRootDataDirectory(NAMESPACE_2);
}

@Test
public void doesNotLoadLeaderPaxosAsNamespace() {
Set<String> namespaces = diskNamespaceLoader.getAllPersistedNamespaces().stream().map(client -> client.value()).collect(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's use a method reference here (Client::value as opposed to x -> x.value())

Collectors.toSet());
assertThat(namespaces).containsExactlyInAnyOrder(NAMESPACE_1, NAMESPACE_2);
}

private void createDirectoryForLeaderForEachClientUseCase(String namespace) {
if (Paths.get(tempFolder.getRoot().toPath().toString(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think it's probably nicer to do tempFolder.getRoot().toPath().resolve(LEADER_PAXOS_NAMESPACE).resolve(...) which should behave in the same way

PaxosTimeLockConstants.LEADER_PAXOS_NAMESPACE,
PaxosTimeLockConstants.MULTI_LEADER_PAXOS_NAMESPACE,
namespace).toFile().mkdirs()) {
return;
}
throw new RuntimeException("Unexpected error when creating a subdirectory");
}

private void createDirectoryInRootDataDirectory(String namespace) {
if (Paths.get(tempFolder.getRoot().toPath().toString(), namespace).toFile().mkdirs()) {
return;
}
throw new RuntimeException("Unexpected error when creating a subdirectory");
}
}