ibessonov commented on code in PR #1640: URL: https://github.com/apache/ignite-3/pull/1640#discussion_r1104174064
########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); Review Comment: I believe you mentioned that this code is for tests only. Why not moving it to test fixtures? ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); + } catch (IOException e) { + throw new NodeConfigReadException("Failed to read config input stream.", e); + } + } + } + return config.get(); + } + }; + } + + /** + * Return node bootstrap configuration with content from {@param plainConf}. + * + * @param plainConf Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration string(@Nullable @Language("HOCON") String plainConf, Path workDir) { + return inputStream(plainConf != null + ? new ByteArrayInputStream(plainConf.getBytes(StandardCharsets.UTF_8)) + : null, + workDir); Review Comment: Looks like an invalid padding ########## modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java: ########## @@ -0,0 +1,38 @@ +/* + * 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.ignite.internal.configuration.storage; + +import java.nio.file.Path; +import java.util.Random; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests for the {@link LocalFileConfigurationStorage}. + */ +public class LocalFileConfigurationStorageTest extends ConfigurationStorageTest { + + @TempDir Review Comment: I wonder why you don't use `@WorkDir` (or whatever that name is, it's our own path provider) ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); + } catch (IOException e) { + throw new NodeConfigReadException("Failed to read config input stream.", e); + } + } + } + return config.get(); Review Comment: I also see that you don't usually have empty lines between statements. What's the reason? I'd prefer Ignite code to look more unified ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); Review Comment: If it's not for tests, then I don't see writing to a temp file ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java: ########## @@ -0,0 +1,241 @@ +/* + * 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.ignite.internal.configuration.storage; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigParseOptions; +import com.typesafe.config.ConfigRenderOptions; +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; +import org.apache.ignite.configuration.annotation.ConfigurationType; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.apache.ignite.internal.configuration.NodeConfigCreateException; +import org.apache.ignite.internal.configuration.NodeConfigWriteException; +import org.apache.ignite.internal.future.InFlightFutures; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; +import org.apache.ignite.internal.util.IgniteUtils; + +/** + * Implementation of {@link ConfigurationStorage} based on local file configuration storage. + */ +public class LocalFileConfigurationStorage implements ConfigurationStorage { + private static final IgniteLogger LOG = Loggers.forClass(LocalFileConfigurationStorage.class); + + /** + * Path to config file. + */ + private final Path configPath; + + /** + * Path to temporary configuration storage. + */ + private final Path tempConfigPath; + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + /** + * Latest state of last applied configuration. + */ + private final Map<String, Serializable> latest = new ConcurrentHashMap<>(); + + /** + * Configuration changes listener. + * */ + private final AtomicReference<ConfigurationStorageListener> lsnrRef = new AtomicReference<>(); + + private final ExecutorService threadPool = Executors.newFixedThreadPool(4, new NamedThreadFactory("loc-cfg-file", LOG)); + + private final InFlightFutures futureTracker = new InFlightFutures(); + + private long lastRevision = 0L; + + /** + * Constructor. + * + * @param configuration Node bootstrap configuration. + */ + public LocalFileConfigurationStorage(NodeBootstrapConfiguration configuration) { + this.configPath = configuration.configPath(); + tempConfigPath = new File(configPath.toFile().getAbsolutePath() + ".storage").toPath(); Review Comment: I think this should work. You don't need to use `File` when you have `Path`, it's a legacy API ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java: ########## @@ -0,0 +1,241 @@ +/* + * 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.ignite.internal.configuration.storage; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigParseOptions; +import com.typesafe.config.ConfigRenderOptions; +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; +import org.apache.ignite.configuration.annotation.ConfigurationType; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.apache.ignite.internal.configuration.NodeConfigCreateException; +import org.apache.ignite.internal.configuration.NodeConfigWriteException; +import org.apache.ignite.internal.future.InFlightFutures; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; +import org.apache.ignite.internal.util.IgniteUtils; + +/** + * Implementation of {@link ConfigurationStorage} based on local file configuration storage. + */ +public class LocalFileConfigurationStorage implements ConfigurationStorage { + private static final IgniteLogger LOG = Loggers.forClass(LocalFileConfigurationStorage.class); + + /** + * Path to config file. + */ + private final Path configPath; + + /** + * Path to temporary configuration storage. + */ + private final Path tempConfigPath; + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + /** + * Latest state of last applied configuration. + */ + private final Map<String, Serializable> latest = new ConcurrentHashMap<>(); + + /** + * Configuration changes listener. + * */ + private final AtomicReference<ConfigurationStorageListener> lsnrRef = new AtomicReference<>(); + + private final ExecutorService threadPool = Executors.newFixedThreadPool(4, new NamedThreadFactory("loc-cfg-file", LOG)); Review Comment: Maybe 2 threads would be enough, I see no realistic scenario where we would need 4 at the same time ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); + } catch (IOException e) { + throw new NodeConfigReadException("Failed to read config input stream.", e); + } + } + } + return config.get(); + } + }; + } + + /** + * Return node bootstrap configuration with content from {@param plainConf}. + * + * @param plainConf Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration string(@Nullable @Language("HOCON") String plainConf, Path workDir) { + return inputStream(plainConf != null + ? new ByteArrayInputStream(plainConf.getBytes(StandardCharsets.UTF_8)) + : null, + workDir); + } + + /** + * Empty config provider. + * + * @param workDir Configuration file location. + * @return Node bootstrap configuration provider to empty config. + */ + static NodeBootstrapConfiguration empty(Path workDir) { + return () -> createEmptyConfig(workDir); + } + + private static Path createEmptyConfig(Path workDir) { + try { + Path config = workDir.resolve("ignite-config.conf"); + File file = config.toFile(); + if (!file.exists()) { + file.createNewFile(); + } + return config; + } catch (IOException e) { + throw new NodeConfigCreateException("Failed to create temp conf file.", e); Review Comment: Why is it a temp file? Name suggests otherwise. And I guess no one deletes it afterwards ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java: ########## @@ -0,0 +1,241 @@ +/* + * 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.ignite.internal.configuration.storage; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigParseOptions; +import com.typesafe.config.ConfigRenderOptions; +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; +import org.apache.ignite.configuration.annotation.ConfigurationType; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.apache.ignite.internal.configuration.NodeConfigCreateException; +import org.apache.ignite.internal.configuration.NodeConfigWriteException; +import org.apache.ignite.internal.future.InFlightFutures; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; +import org.apache.ignite.internal.util.IgniteUtils; + +/** + * Implementation of {@link ConfigurationStorage} based on local file configuration storage. + */ +public class LocalFileConfigurationStorage implements ConfigurationStorage { + private static final IgniteLogger LOG = Loggers.forClass(LocalFileConfigurationStorage.class); + + /** + * Path to config file. + */ + private final Path configPath; + + /** + * Path to temporary configuration storage. + */ + private final Path tempConfigPath; + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + /** + * Latest state of last applied configuration. + */ + private final Map<String, Serializable> latest = new ConcurrentHashMap<>(); + + /** + * Configuration changes listener. + * */ + private final AtomicReference<ConfigurationStorageListener> lsnrRef = new AtomicReference<>(); + + private final ExecutorService threadPool = Executors.newFixedThreadPool(4, new NamedThreadFactory("loc-cfg-file", LOG)); + + private final InFlightFutures futureTracker = new InFlightFutures(); + + private long lastRevision = 0L; + + /** + * Constructor. + * + * @param configuration Node bootstrap configuration. + */ + public LocalFileConfigurationStorage(NodeBootstrapConfiguration configuration) { + this.configPath = configuration.configPath(); + tempConfigPath = new File(configPath.toFile().getAbsolutePath() + ".storage").toPath(); Review Comment: ```suggestion tempConfigPath = configPath.resolveSibling(configPath.getFileName() + ".storage"); ``` ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path configPath(); + + /** + * Simple config file provider. + * + * @param configPath Path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is Configuration content. + * @param workDir Dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path configPath() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes(), StandardOpenOption.DSYNC); + } catch (IOException e) { + throw new NodeConfigReadException("Failed to read config input stream.", e); Review Comment: What if it was an error during the write? They're both in the same "try" section ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java: ########## @@ -0,0 +1,241 @@ +/* + * 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.ignite.internal.configuration.storage; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigParseOptions; +import com.typesafe.config.ConfigRenderOptions; +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; +import org.apache.ignite.configuration.annotation.ConfigurationType; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.apache.ignite.internal.configuration.NodeConfigCreateException; +import org.apache.ignite.internal.configuration.NodeConfigWriteException; +import org.apache.ignite.internal.future.InFlightFutures; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; +import org.apache.ignite.internal.util.IgniteUtils; + +/** + * Implementation of {@link ConfigurationStorage} based on local file configuration storage. + */ +public class LocalFileConfigurationStorage implements ConfigurationStorage { + private static final IgniteLogger LOG = Loggers.forClass(LocalFileConfigurationStorage.class); + + /** + * Path to config file. + */ + private final Path configPath; + + /** + * Path to temporary configuration storage. + */ + private final Path tempConfigPath; + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + /** + * Latest state of last applied configuration. + */ + private final Map<String, Serializable> latest = new ConcurrentHashMap<>(); + + /** + * Configuration changes listener. + * */ + private final AtomicReference<ConfigurationStorageListener> lsnrRef = new AtomicReference<>(); + + private final ExecutorService threadPool = Executors.newFixedThreadPool(4, new NamedThreadFactory("loc-cfg-file", LOG)); + + private final InFlightFutures futureTracker = new InFlightFutures(); + + private long lastRevision = 0L; + + /** + * Constructor. + * + * @param configuration Node bootstrap configuration. + */ + public LocalFileConfigurationStorage(NodeBootstrapConfiguration configuration) { + this.configPath = configuration.configPath(); + tempConfigPath = new File(configPath.toFile().getAbsolutePath() + ".storage").toPath(); Review Comment: Why not `.tmp`? ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java: ########## @@ -0,0 +1,241 @@ +/* + * 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.ignite.internal.configuration.storage; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigParseOptions; +import com.typesafe.config.ConfigRenderOptions; +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; +import org.apache.ignite.configuration.annotation.ConfigurationType; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.apache.ignite.internal.configuration.NodeConfigCreateException; +import org.apache.ignite.internal.configuration.NodeConfigWriteException; +import org.apache.ignite.internal.future.InFlightFutures; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; +import org.apache.ignite.internal.util.IgniteUtils; + +/** + * Implementation of {@link ConfigurationStorage} based on local file configuration storage. + */ +public class LocalFileConfigurationStorage implements ConfigurationStorage { + private static final IgniteLogger LOG = Loggers.forClass(LocalFileConfigurationStorage.class); + + /** + * Path to config file. + */ + private final Path configPath; + + /** + * Path to temporary configuration storage. + */ + private final Path tempConfigPath; + + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + + /** + * Latest state of last applied configuration. + */ + private final Map<String, Serializable> latest = new ConcurrentHashMap<>(); + + /** + * Configuration changes listener. + * */ + private final AtomicReference<ConfigurationStorageListener> lsnrRef = new AtomicReference<>(); + + private final ExecutorService threadPool = Executors.newFixedThreadPool(4, new NamedThreadFactory("loc-cfg-file", LOG)); + + private final InFlightFutures futureTracker = new InFlightFutures(); + + private long lastRevision = 0L; + + /** + * Constructor. + * + * @param configuration Node bootstrap configuration. + */ + public LocalFileConfigurationStorage(NodeBootstrapConfiguration configuration) { + this.configPath = configuration.configPath(); + tempConfigPath = new File(configPath.toFile().getAbsolutePath() + ".storage").toPath(); + checkAndRestoreConfigFile(); + } + + @Override + public CompletableFuture<Data> readDataOnRecovery() { + return CompletableFuture.completedFuture(new Data(Collections.emptyMap(), 0)); + } + + @Override + public CompletableFuture<Map<String, ? extends Serializable>> readAllLatest(String prefix) { + lock.readLock().lock(); + try { + checkAndRestoreConfigFile(); + Map<String, Serializable> map = latest.entrySet() + .stream() + .filter(entry -> entry.getKey().startsWith(prefix)) + .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); + return CompletableFuture.completedFuture(map); + } finally { + lock.readLock().unlock(); + } + } + + @Override + public CompletableFuture<Serializable> readLatest(String key) { + lock.readLock().lock(); + try { + checkAndRestoreConfigFile(); + return CompletableFuture.completedFuture(latest.get(key)); + } finally { + lock.readLock().unlock(); + } + } + + @Override + public CompletableFuture<Boolean> write(Map<String, ? extends Serializable> newValues, long ver) { + lock.writeLock().lock(); + try { + if (ver != lastRevision) { + return CompletableFuture.completedFuture(false); + } + checkAndRestoreConfigFile(); + saveValues(newValues); + latest.putAll(newValues); + lastRevision++; + runAsync(() -> lsnrRef.get().onEntriesChanged(new Data(newValues, lastRevision))); + return CompletableFuture.completedFuture(true); + } finally { + lock.writeLock().unlock(); + } + } + + private void runAsync(Runnable runnable) { + CompletableFuture<Void> future = CompletableFuture.runAsync(runnable, threadPool); + + futureTracker.registerFuture(future); + } + + @Override + public void registerConfigurationListener(ConfigurationStorageListener lsnr) { + if (!lsnrRef.compareAndSet(null, lsnr)) { + LOG.debug("Configuration listener has already been set"); + } + } + + @Override + public ConfigurationType type() { + return ConfigurationType.LOCAL; + } + + @Override + public CompletableFuture<Long> lastRevision() { + return CompletableFuture.completedFuture(lastRevision); + } + + @Override + public CompletableFuture<Void> writeConfigurationRevision(long prevRevision, long currentRevision) { + return CompletableFuture.completedFuture(null); + } + + @Override + public void close() { + IgniteUtils.shutdownAndAwaitTermination(threadPool, 10, TimeUnit.SECONDS); + + futureTracker.cancelInFlightFutures(); + } + + private void saveValues(Map<String, ? extends Serializable> values) { + try { + String s = renderHoconString(values); + Files.write(tempConfigPath, s.getBytes(StandardCharsets.UTF_8), StandardOpenOption.DSYNC); Review Comment: Why `DSYNC` and not `SYNC`? ########## modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java: ########## @@ -0,0 +1,38 @@ +/* + * 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.ignite.internal.configuration.storage; + +import java.nio.file.Path; +import java.util.Random; +import org.apache.ignite.internal.configuration.NodeBootstrapConfiguration; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests for the {@link LocalFileConfigurationStorage}. + */ +public class LocalFileConfigurationStorageTest extends ConfigurationStorageTest { + + @TempDir + private Path tmpDir; + + @Override + public ConfigurationStorage getStorage() { + Path resolve = tmpDir.resolve("file" + new Random().nextInt()); Review Comment: And why does it have to be a random name. Are you trying to prevent collisions? Looks suspicious. ########## modules/runner/src/main/java/org/apache/ignite/internal/configuration/NodeBootstrapConfiguration.java: ########## @@ -0,0 +1,119 @@ +/* + * 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.ignite.internal.configuration; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicReference; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Node bootstrap configuration provider interface. + */ +@FunctionalInterface +public interface NodeBootstrapConfiguration { + /** + * Path to node configuration file. + * + * @return Path to node configuration file in HOCON format. + */ + Path config(); + + + /** + * Simple config file provider. + * + * @param configPath path to node bootstrap configuration. + * @return Simple implementation with provided configuration file. + */ + static NodeBootstrapConfiguration directFile(@NotNull Path configPath) { + return () -> configPath; + } + + /** + * Return node bootstrap configuration with content from {@param is}. + * + * @param is configuration content. + * @param workDir dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration inputStream(@Nullable InputStream is, Path workDir) { + return new NodeBootstrapConfiguration() { + + private final AtomicReference<Path> config = new AtomicReference<>(); + + @Override + public Path config() { + if (config.compareAndSet(null, createEmptyConfig(workDir))) { + if (is != null) { + try { + Files.write(config.get(), is.readAllBytes()); + } catch (IOException e) { + throw new NodeConfigReadException("Failed to read config input stream.", e); + } + } + } + return config.get(); + } + }; + } + + /** + * Return node bootstrap configuration with content from {@param plainConf}. + * + * @param plainConf configuration content. + * @param workDir dir for configuration file location. + * @return Node bootstrap configuration with lazy config file creation. + */ + static NodeBootstrapConfiguration string(@Nullable @Language("HOCON") String plainConf, Path workDir) { + return inputStream(plainConf != null + ? new ByteArrayInputStream(plainConf.getBytes(StandardCharsets.UTF_8)) + : null, + workDir); + } + + /** + * Empty config provider. + * + * @param workDir Configuration file location. + * @return Node bootstrap configuration provider to empty config. + */ + static NodeBootstrapConfiguration empty(Path workDir) { + return () -> createEmptyConfig(workDir); + } + + private static Path createEmptyConfig(Path workDir) { + try { + Path config = workDir.resolve("ignite-config.conf"); Review Comment: Ok. Can we have a constant for this string? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
