Skip to content

Commit

Permalink
Revert "Fix that config changes rejected by DN will be written to CN'…
Browse files Browse the repository at this point in the history 
…s config (#14551)" (#14590)

This reverts commit 2c4bb24.
  • Loading branch information
@shuwenwei
shuwenwei authored Dec 30, 2024
1 parent a896d92 commit 2de80ba
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 76 deletions.
17 changes: 4 additions & 13 deletions integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
@RunWith(IoTDBTestRunner.class)
@Category({LocalStandaloneIT.class})
public class IoTDBSetConfigurationIT {

@BeforeClass
public static void setUp() throws Exception {
EnvFactory.getEnv().initClusterEnvironment();
Expand Down Expand Up @@ -109,8 +108,6 @@ public void testSetConfiguration() {
} catch (Exception e) {
Assert.fail(e.getMessage());
}
// set configuration "enable_seq_space_compaction"="false"
// set configuration "enable_unseq_space_compaction"="false" on 0
Assert.assertTrue(
EnvFactory.getEnv().getConfigNodeWrapperList().stream()
.allMatch(
Expand All @@ -119,17 +116,14 @@ public void testSetConfiguration() {
nodeWrapper,
"enable_seq_space_compaction=false",
"enable_unseq_space_compaction=false")));
// set configuration "enable_seq_space_compaction"="false"
Assert.assertTrue(
EnvFactory.getEnv().getDataNodeWrapperList().stream()
.allMatch(
nodeWrapper ->
checkConfigFileContains(nodeWrapper, "enable_seq_space_compaction=false")));
// set configuration "enable_cross_space_compaction"="false" on 1
assertTrue(
checkConfigFileContains(
EnvFactory.getEnv().getDataNodeWrapperList().get(0),
"enable_cross_space_compaction=false"));
checkConfigFileContains(
nodeWrapper,
"enable_seq_space_compaction=false",
"enable_cross_space_compaction=false")));
}

@Test
Expand Down Expand Up @@ -246,9 +240,6 @@ public void testSetDefaultSGLevel() throws SQLException {
assertFalse(
checkConfigFileContains(
EnvFactory.getEnv().getDataNodeWrapper(0), "default_storage_group_level=-1"));
assertFalse(
checkConfigFileContains(
EnvFactory.getEnv().getConfigNodeWrapper(0), "default_storage_group_level=-1"));
assertTrue(
checkConfigFileContains(
EnvFactory.getEnv().getDataNodeWrapper(0), "default_storage_group_level=3"));
Expand Down
76 changes: 27 additions & 49 deletions iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.apache.iotdb.commons.conf.IoTDBConstant.ONE_LEVEL_PATH_WILDCARD;
Expand Down Expand Up @@ -1580,62 +1579,41 @@ public TSStatus setConfiguration(TSetConfigurationReq req) {
return tsStatus;
}
}
if (currentNodeId == req.getNodeId() || req.getNodeId() < 0) {
URL url = ConfigNodeDescriptor.getPropsUrl(CommonConfig.SYSTEM_CONFIG_NAME);
boolean configurationFileFound = (url != null && new File(url.getFile()).exists());
TrimProperties properties = new TrimProperties();
properties.putAll(req.getConfigs());

if (currentNodeId == req.getNodeId()) {
return setConfigLocally(req, null);
} else if (req.getNodeId() < 0) {
// re-config CN in memory -> re-config DN in memory -> re-config DN in file -> re-config CN in
// file
TSStatus finalTsStatus = tsStatus;
return setConfigLocally(req, () -> broadcastSetConfig(finalTsStatus, req));
} else {
// not for this node, ignore it
return broadcastSetConfig(tsStatus, req);
if (configurationFileFound) {
File file = new File(url.getFile());
try {
ConfigurationFileUtils.updateConfiguration(
file,
properties,
mergedProps -> {
ConfigNodeDescriptor.getInstance().loadHotModifiedProps(mergedProps);
});
} catch (Exception e) {
tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, e.getMessage());
}
} else {
String msg =
"Unable to find the configuration file. Some modifications are made only in memory.";
tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, msg);
LOGGER.warn(msg);
}
if (currentNodeId == req.getNodeId()) {
return tsStatus;
}
}
}

private TSStatus broadcastSetConfig(TSStatus thisNodeResult, TSetConfigurationReq req) {
List<TSStatus> statusListOfOtherNodes = nodeManager.setConfiguration(req);
List<TSStatus> statusList = new ArrayList<>(statusListOfOtherNodes.size() + 1);
statusList.add(thisNodeResult);
statusList.add(tsStatus);
statusList.addAll(statusListOfOtherNodes);
return RpcUtils.squashResponseStatusList(statusList);
}

private TSStatus setConfigLocally(
TSetConfigurationReq req, Supplier<TSStatus> beforeWriteFileAction) {
// re-config this node only
TSStatus tsStatus;
URL url = ConfigNodeDescriptor.getPropsUrl(CommonConfig.SYSTEM_CONFIG_NAME);
boolean configurationFileFound = (url != null && new File(url.getFile()).exists());
TrimProperties newProperties = new TrimProperties();
newProperties.putAll(req.getConfigs());

if (configurationFileFound) {
File file = new File(url.getFile());
try {
tsStatus =
ConfigurationFileUtils.updateConfiguration(
file,
newProperties,
mergedProps -> {
ConfigNodeDescriptor.getInstance().loadHotModifiedProps(mergedProps);
return beforeWriteFileAction != null
? beforeWriteFileAction.get()
: StatusUtils.OK;
});
} catch (Exception e) {
tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, e.getMessage());
}
} else {
String msg =
"Unable to find the configuration file. Some modifications are made only in memory.";
tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, msg);
LOGGER.warn(msg);
}
return tsStatus;
}

@Override
public TSStatus startRepairData() {
TSStatus status = confirmLeader();
Expand Down
2 changes: 0 additions & 2 deletions iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.apache.iotdb.commons.service.IService;
import org.apache.iotdb.commons.service.ServiceType;
import org.apache.iotdb.commons.utils.PathUtils;
import org.apache.iotdb.commons.utils.StatusUtils;
import org.apache.iotdb.commons.utils.TestOnly;
import org.apache.iotdb.commons.utils.TimePartitionUtils;
import org.apache.iotdb.consensus.ConsensusFactory;
Expand Down Expand Up @@ -687,7 +686,6 @@ public TSStatus setConfiguration(TSetConfigurationReq req) {
} catch (Exception e) {
throw new IllegalArgumentException(e);
}
return StatusUtils.OK;
});
} catch (Exception e) {
if (e instanceof InterruptedException) {
Expand Down
16 changes: 4 additions & 12 deletions ...core/node-commons/src/main/java/org/apache/iotdb/commons/conf/ConfigurationFileUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@

package org.apache.iotdb.commons.conf;

import org.apache.iotdb.common.rpc.thrift.TSStatus;
import org.apache.iotdb.commons.utils.StatusUtils;
import org.apache.iotdb.rpc.TSStatusCode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -252,7 +248,7 @@ public static List<String> filterInvalidConfigItems(Map<String, String> configIt
return ignoredConfigItems;
}

public static TSStatus updateConfiguration(
public static void updateConfiguration(
File file, Properties newConfigItems, LoadHotModifiedPropsFunc loadHotModifiedPropertiesFunc)
throws IOException, InterruptedException {
File lockFile = new File(file.getPath() + lockFileSuffix);
Expand All @@ -275,10 +271,7 @@ public static TSStatus updateConfiguration(

// load hot modified properties
if (loadHotModifiedPropertiesFunc != null) {
TSStatus status = loadHotModifiedPropertiesFunc.loadHotModifiedProperties(mergedProps);
if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
return status;
}
loadHotModifiedPropertiesFunc.loadHotModifiedProperties(mergedProps);
}

// generate new configuration file content in memory
Expand Down Expand Up @@ -307,7 +300,7 @@ public static TSStatus updateConfiguration(
}
if (newConfigItems.isEmpty()) {
// No configuration needs to be modified
return StatusUtils.OK;
return;
}
logger.info("Updating configuration file {}", file.getAbsolutePath());
try (BufferedWriter writer = new BufferedWriter(new FileWriter(lockFile))) {
Expand All @@ -323,7 +316,6 @@ public static TSStatus updateConfiguration(
} finally {
releaseFileLock(lockFile);
}
return StatusUtils.OK;
}

private static String readConfigLinesWithoutLicense(File file) throws IOException {
Expand Down Expand Up @@ -366,7 +358,7 @@ private static void releaseFileLock(File file) throws IOException {

@FunctionalInterface
public interface LoadHotModifiedPropsFunc {
TSStatus loadHotModifiedProperties(TrimProperties properties)
void loadHotModifiedProperties(TrimProperties properties)
throws IOException, InterruptedException;
}
}

0 comments on commit 2de80ba

Please sign in to comment.