yifan-c commented on code in PR #212: URL: https://github.com/apache/cassandra-sidecar/pull/212#discussion_r2117489094
########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/CassandraInstanceFilesImpl.java: ########## @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.response.InstanceFileInfo; +import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration; + +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.hasAnyPlaceholder; +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder; + +/** + * Helper class to get the list of files to use during Live Migration. + */ +public class CassandraInstanceFilesImpl implements CassandraInstanceFiles +{ + + private static final Logger LOGGER = LoggerFactory.getLogger(CassandraInstanceFilesImpl.class); + + private final InstanceMetadata instanceMetadata; + + private final LiveMigrationConfiguration configuration; + + public CassandraInstanceFilesImpl(InstanceMetadata instanceMetadata, + LiveMigrationConfiguration configuration) + { + this.instanceMetadata = instanceMetadata; + this.configuration = configuration; + } + + @Override + public List<InstanceFileInfo> getFiles() throws IOException + { + return getFiles(configuration.filesToExclude(), configuration.directoriesToExclude()); + } + + private List<InstanceFileInfo> getFiles(Set<String> filesToExclude, + Set<String> dirsToExclude) throws IOException + { + List<DirVisitor> dirVisitors = getDirVisitorList(filesToExclude, dirsToExclude); + List<InstanceFileInfo> instanceFileInfos = new ArrayList<>(); + + for (DirVisitor dirVisitor : dirVisitors) + { + instanceFileInfos.addAll(dirVisitor.getFiles()); + } + + LOGGER.info("{} number of files are getting listed", instanceFileInfos.size()); + return instanceFileInfos; + } + + public List<DirVisitor> getDirVisitorList(Set<String> filesToExclude, + Set<String> dirsToExclude) + { + List<DirVisitor> dataFilesToVisit = new ArrayList<>(); + List<String> dirsToCopy = LiveMigrationInstanceMetadataUtil.dirsToCopy(instanceMetadata); + Map<String, String> dirPathPrefix = LiveMigrationInstanceMetadataUtil.dirPathPrefixMap(instanceMetadata); + Map<String, Set<String>> dirPlaceholderMap = LiveMigrationInstanceMetadataUtil.dirPlaceHoldersMap(instanceMetadata); + + for (String dir : dirsToCopy) + { + getDirToVisit(dir, + dirPlaceholderMap.get(dir), + dirPathPrefix.get(dir), + filesToExclude, + dirsToExclude) + .ifPresent(dataFilesToVisit::add); + } + + return dataFilesToVisit; + } + + Optional<DirVisitor> getDirToVisit(String homeDir, Set<String> placeholders, String pathPrefix, + Set<String> filesToExclude, Set<String> dirsToExclude) + { + if (null == homeDir) + { + return Optional.empty(); + } + Path homeDirPath = Paths.get(homeDir); + if (!Files.exists(homeDirPath)) + { + LOGGER.info("Directory {} does not exist.", homeDir); + return Optional.empty(); + } + + Objects.requireNonNull(placeholders, "placeholders are required"); + Objects.requireNonNull(pathPrefix, "pathPrefix is required"); + + Set<PathMatcher> fileExclusionMatchers = toPathMatchers(filesToExclude, placeholders, homeDir); + Set<PathMatcher> dirExclusionMatchers = toPathMatchers(dirsToExclude, placeholders, homeDir); + + return Optional.of(new DirVisitor(homeDir, pathPrefix, fileExclusionMatchers, dirExclusionMatchers)); + } + + Set<PathMatcher> toPathMatchers(Set<String> exclusions, Set<String> homeDirPlaceholders, String homeDir) + { + if (null == exclusions) Review Comment: nit: check empty case too. ```suggestion if (null == exclusions || exclusions.isEmpty()) ``` ########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/CassandraInstanceFilesImpl.java: ########## @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.response.InstanceFileInfo; +import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration; + +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.hasAnyPlaceholder; +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder; + +/** + * Helper class to get the list of files to use during Live Migration. + */ +public class CassandraInstanceFilesImpl implements CassandraInstanceFiles +{ + + private static final Logger LOGGER = LoggerFactory.getLogger(CassandraInstanceFilesImpl.class); + + private final InstanceMetadata instanceMetadata; + + private final LiveMigrationConfiguration configuration; + + public CassandraInstanceFilesImpl(InstanceMetadata instanceMetadata, + LiveMigrationConfiguration configuration) + { + this.instanceMetadata = instanceMetadata; + this.configuration = configuration; + } + + @Override + public List<InstanceFileInfo> getFiles() throws IOException + { + return getFiles(configuration.filesToExclude(), configuration.directoriesToExclude()); + } + + private List<InstanceFileInfo> getFiles(Set<String> filesToExclude, + Set<String> dirsToExclude) throws IOException + { + List<DirVisitor> dirVisitors = getDirVisitorList(filesToExclude, dirsToExclude); + List<InstanceFileInfo> instanceFileInfos = new ArrayList<>(); + + for (DirVisitor dirVisitor : dirVisitors) + { + instanceFileInfos.addAll(dirVisitor.getFiles()); + } + + LOGGER.info("{} number of files are getting listed", instanceFileInfos.size()); + return instanceFileInfos; + } + + public List<DirVisitor> getDirVisitorList(Set<String> filesToExclude, + Set<String> dirsToExclude) + { + List<DirVisitor> dataFilesToVisit = new ArrayList<>(); + List<String> dirsToCopy = LiveMigrationInstanceMetadataUtil.dirsToCopy(instanceMetadata); + Map<String, String> dirPathPrefix = LiveMigrationInstanceMetadataUtil.dirPathPrefixMap(instanceMetadata); + Map<String, Set<String>> dirPlaceholderMap = LiveMigrationInstanceMetadataUtil.dirPlaceHoldersMap(instanceMetadata); + + for (String dir : dirsToCopy) + { + getDirToVisit(dir, + dirPlaceholderMap.get(dir), + dirPathPrefix.get(dir), + filesToExclude, + dirsToExclude) + .ifPresent(dataFilesToVisit::add); + } + + return dataFilesToVisit; + } + + Optional<DirVisitor> getDirToVisit(String homeDir, Set<String> placeholders, String pathPrefix, + Set<String> filesToExclude, Set<String> dirsToExclude) + { + if (null == homeDir) + { + return Optional.empty(); + } + Path homeDirPath = Paths.get(homeDir); + if (!Files.exists(homeDirPath)) + { + LOGGER.info("Directory {} does not exist.", homeDir); + return Optional.empty(); + } + + Objects.requireNonNull(placeholders, "placeholders are required"); + Objects.requireNonNull(pathPrefix, "pathPrefix is required"); + + Set<PathMatcher> fileExclusionMatchers = toPathMatchers(filesToExclude, placeholders, homeDir); + Set<PathMatcher> dirExclusionMatchers = toPathMatchers(dirsToExclude, placeholders, homeDir); + + return Optional.of(new DirVisitor(homeDir, pathPrefix, fileExclusionMatchers, dirExclusionMatchers)); + } + + Set<PathMatcher> toPathMatchers(Set<String> exclusions, Set<String> homeDirPlaceholders, String homeDir) Review Comment: I should probably make the comment here on the usage of `set`. ########## conf/sidecar.yaml: ########## @@ -362,3 +362,10 @@ s3_client: # uri: # username: # password: + +live_migration: + files_to_exclude: [] + dirs_to_exclude: + - ${DATA_FILE_DIR}/*/*/snapshots # Excludes snapshot directories to copy to destination Review Comment: can you please also add documents regarding the other available "placeholders"? Basically, we need to let end users know how to configure w/o having to look at the code. ########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/CassandraInstanceFilesImpl.java: ########## @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.response.InstanceFileInfo; +import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration; + +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.hasAnyPlaceholder; +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder; + +/** + * Helper class to get the list of files to use during Live Migration. + */ +public class CassandraInstanceFilesImpl implements CassandraInstanceFiles +{ + + private static final Logger LOGGER = LoggerFactory.getLogger(CassandraInstanceFilesImpl.class); + + private final InstanceMetadata instanceMetadata; + + private final LiveMigrationConfiguration configuration; + + public CassandraInstanceFilesImpl(InstanceMetadata instanceMetadata, + LiveMigrationConfiguration configuration) + { + this.instanceMetadata = instanceMetadata; + this.configuration = configuration; + } + + @Override + public List<InstanceFileInfo> getFiles() throws IOException Review Comment: nit: the code style prefer omitting `get` from `getters`. I will _not_ insist if you feel the current name is fine. But, if you do want to update, please take care of the other getters, e.g. `getDirVisitorList` in this class. ########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/CassandraInstanceFilesImpl.java: ########## @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.response.InstanceFileInfo; +import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration; + +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.hasAnyPlaceholder; +import static org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder; + +/** + * Helper class to get the list of files to use during Live Migration. + */ +public class CassandraInstanceFilesImpl implements CassandraInstanceFiles +{ + + private static final Logger LOGGER = LoggerFactory.getLogger(CassandraInstanceFilesImpl.class); + + private final InstanceMetadata instanceMetadata; + + private final LiveMigrationConfiguration configuration; + + public CassandraInstanceFilesImpl(InstanceMetadata instanceMetadata, + LiveMigrationConfiguration configuration) + { + this.instanceMetadata = instanceMetadata; + this.configuration = configuration; + } + + @Override + public List<InstanceFileInfo> getFiles() throws IOException + { + return getFiles(configuration.filesToExclude(), configuration.directoriesToExclude()); + } + + private List<InstanceFileInfo> getFiles(Set<String> filesToExclude, + Set<String> dirsToExclude) throws IOException + { + List<DirVisitor> dirVisitors = getDirVisitorList(filesToExclude, dirsToExclude); + List<InstanceFileInfo> instanceFileInfos = new ArrayList<>(); + + for (DirVisitor dirVisitor : dirVisitors) + { + instanceFileInfos.addAll(dirVisitor.getFiles()); + } + + LOGGER.info("{} number of files are getting listed", instanceFileInfos.size()); + return instanceFileInfos; + } + + public List<DirVisitor> getDirVisitorList(Set<String> filesToExclude, + Set<String> dirsToExclude) + { + List<DirVisitor> dataFilesToVisit = new ArrayList<>(); + List<String> dirsToCopy = LiveMigrationInstanceMetadataUtil.dirsToCopy(instanceMetadata); + Map<String, String> dirPathPrefix = LiveMigrationInstanceMetadataUtil.dirPathPrefixMap(instanceMetadata); + Map<String, Set<String>> dirPlaceholderMap = LiveMigrationInstanceMetadataUtil.dirPlaceHoldersMap(instanceMetadata); + + for (String dir : dirsToCopy) + { + getDirToVisit(dir, + dirPlaceholderMap.get(dir), + dirPathPrefix.get(dir), + filesToExclude, + dirsToExclude) + .ifPresent(dataFilesToVisit::add); + } + + return dataFilesToVisit; + } + + Optional<DirVisitor> getDirToVisit(String homeDir, Set<String> placeholders, String pathPrefix, + Set<String> filesToExclude, Set<String> dirsToExclude) + { + if (null == homeDir) + { + return Optional.empty(); + } + Path homeDirPath = Paths.get(homeDir); + if (!Files.exists(homeDirPath)) + { + LOGGER.info("Directory {} does not exist.", homeDir); + return Optional.empty(); + } + + Objects.requireNonNull(placeholders, "placeholders are required"); + Objects.requireNonNull(pathPrefix, "pathPrefix is required"); Review Comment: nit: move those 2 checks before the files existence check; can save a bit of I/O if any of the condition fails. ########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationPlaceholderUtil.java: ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.jetbrains.annotations.Nullable; + +/** + * Utility class for replacing placeholder in Live Migration exclusion paths. + * <p> + * A placeholder looks like this: "${PLACEHOLDER}/*\/*\/*-Data.db". + */ +public class LiveMigrationPlaceholderUtil +{ + + public static final String DATA_FILE_DIR_PLACEHOLDER = "DATA_FILE_DIR"; + public static final String COMMITLOG_DIR_PLACEHOLDER = "COMMITLOG_DIR"; + public static final String HINTS_DIR_PLACEHOLDER = "HINTS_DIR"; + public static final String SAVED_CACHES_DIR_PLACEHOLDER = "SAVED_CACHES_DIR"; + public static final String CDC_RAW_DIR_PLACEHOLDER = "CDC_RAW_DIR"; + public static final String LOCAL_SYSTEM_DATA_FILE_DIR_PLACEHOLDER = "LOCAL_SYSTEM_DATA_FILE_DIR"; Review Comment: nit: they could be enum. It would make contains check easier to reason about. ########## server/src/main/java/org/apache/cassandra/sidecar/livemigration/MigrationFileVisitor.java: ########## @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cassandra.sidecar.livemigration; + +import java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.jetbrains.annotations.NotNull; + +/** + * File visitor to walk through given directory. + */ +public class MigrationFileVisitor extends SimpleFileVisitor<Path> +{ + private static final Logger LOGGER = LoggerFactory.getLogger(MigrationFileVisitor.class); + private final String homeDir; + private final List<Path> validFiles; + private final Set<PathMatcher> filesToExclude; + private final Set<PathMatcher> directoriesToExclude; + + public MigrationFileVisitor(@NotNull final String homeDir, + @NotNull final Set<PathMatcher> filesToExcludeMatchers, + @NotNull final Set<PathMatcher> directoriesToExcludeMatchers) Review Comment: 1. remove `final`s 2. I think there is no guarantee that implementation of `PathMatcher` implements hashcode and equals. I am not sure of the benefit of using a set to hold the `PathMatcher`s. I think a list would just do fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
