You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "choubenson (via GitHub)" <gi...@apache.org> on 2023/06/07 09:43:23 UTC
[GitHub] [iotdb] choubenson opened a new pull request, #10069: [IOTDB-5972]Limit the number of all source files in compaction
choubenson opened a new pull request, #10069:
URL: https://github.com/apache/iotdb/pull/10069
Control the number of files of all compaction threads in the system to prevent too many open file handles.
Now we have control the number of files in a single compaction task, but in cross-space compaction, if an unseq file has a large time span and overlaps with too many seq files, it will increase the number of source files. The improvement method is: control the total number of source files of the compaction threads in the system, check the number of files before the task starts, wait if it exceeds the threshold, and exit after timeout.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr merged pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr merged PR #10069:
URL: https://github.com/apache/iotdb/pull/10069
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223774956
##########
server/src/main/java/org/apache/iotdb/db/rescon/SystemInfo.java:
##########
@@ -54,6 +56,11 @@ public class SystemInfo {
private long flushingMemTablesCost = 0L;
private AtomicLong compactionMemoryCost = new AtomicLong(0L);
+ private AtomicInteger compactionFileNumCost = new AtomicInteger(0);
+
+ private int maxFileNumForCompaction =
+ config.getMaxCrossCompactionCandidateFileNum() * config.getCompactionThreadCount();
Review Comment:
Actually we already have a hard limitation to make total file count in one cross task is no more than getMaxCrossCompactionCandidateFileNum(), is this var `maxFileNumForCompaction` necessary ?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] choubenson commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "choubenson (via GitHub)" <gi...@apache.org>.
choubenson commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223790761
##########
server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/exception/CompactionFileNumNotEnoughException.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * 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.iotdb.db.engine.compaction.execute.exception;
+
+public class CompactionFileNumNotEnoughException extends Exception {
Review Comment:
Resolved.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223772369
##########
server/src/main/java/org/apache/iotdb/db/rescon/SystemInfo.java:
##########
@@ -215,6 +246,10 @@ public synchronized void resetCompactionMemoryCost(long compactionMemoryCost) {
this.compactionMemoryCost.addAndGet(-compactionMemoryCost);
}
+ public synchronized void resetCompactionFileNumCost(int fileNum) {
Review Comment:
suggest to rename it to `decreaseCompactionFileNumCost`
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] choubenson commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "choubenson (via GitHub)" <gi...@apache.org>.
choubenson commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223790449
##########
server/src/main/java/org/apache/iotdb/db/rescon/SystemInfo.java:
##########
@@ -215,6 +246,10 @@ public synchronized void resetCompactionMemoryCost(long compactionMemoryCost) {
this.compactionMemoryCost.addAndGet(-compactionMemoryCost);
}
+ public synchronized void resetCompactionFileNumCost(int fileNum) {
Review Comment:
Resolved.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1222831211
##########
server/src/main/java/org/apache/iotdb/db/rescon/SystemInfo.java:
##########
@@ -54,6 +56,11 @@ public class SystemInfo {
private long flushingMemTablesCost = 0L;
private AtomicLong compactionMemoryCost = new AtomicLong(0L);
+ private AtomicInteger compactionFileNumCost = new AtomicInteger(0);
+
+ private int maxFileNumForCompaction =
+ config.getMaxCrossCompactionCandidateFileNum() * config.getCompactionThreadCount();
Review Comment:
Can we use a totalNum in simple rather than calculated by CompactionThreadCount ?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] choubenson commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "choubenson (via GitHub)" <gi...@apache.org>.
choubenson commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223966792
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
Review Comment:
Resolved.
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
+
+ /** The max candidate file num in compaction */
+ private int maxCompactionCandidateFileNum = 5000;
Review Comment:
Resolved.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223898786
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
+
+ /** The max candidate file num in compaction */
+ private int maxCompactionCandidateFileNum = 5000;
Review Comment:
Suggest to rename to `totalFileLimitForCrossTask`
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
Review Comment:
Suggest to change to `fileLimitPerCrossTask`
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223894194
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
Review Comment:
Suggest to change to `SingleCrossTaskCandidateFileLimit`
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223769686
##########
docs/zh/UserGuide/Reference/Common-Config-Manual.md:
##########
@@ -1023,11 +1023,11 @@ IoTDB ConfigNode 和 DataNode 的公共配置参数位于 `conf` 目录下。
* max\_cross\_compaction\_candidate\_file\_num
|名字| max\_cross\_compaction\_candidate\_file\_num |
-|:---:|:---|
-|描述| 跨空间合并中一次合并最多参与的文件数 |
-|类型| int32 |
-|默认值| 1000|
-|改后生效方式|重启服务生效|
+|:---:|:---------------------------------------------|
+|描述| 跨空间合并中一次合并最多参与的文件数 |
Review Comment:
In current implementation, the parameter is used to represent total files in all cross compaction task rather than one cross compaction task
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223770270
##########
docs/zh/UserGuide/Reference/Common-Config-Manual.md:
##########
@@ -1023,11 +1023,11 @@ IoTDB ConfigNode 和 DataNode 的公共配置参数位于 `conf` 目录下。
* max\_cross\_compaction\_candidate\_file\_num
|名字| max\_cross\_compaction\_candidate\_file\_num |
-|:---:|:---|
-|描述| 跨空间合并中一次合并最多参与的文件数 |
-|类型| int32 |
-|默认值| 1000|
-|改后生效方式|重启服务生效|
+|:---:|:---------------------------------------------|
+|描述| 跨空间合并中一次合并最多参与的文件数 |
Review Comment:
I misunderstood this...but... its description is still not accurate
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1223894194
##########
server/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java:
##########
@@ -498,7 +498,10 @@ public class IoTDBConfig {
private int maxInnerCompactionCandidateFileNum = 30;
/** The max candidate file num in cross space compaction */
- private int maxCrossCompactionCandidateFileNum = 1000;
+ private int maxCrossCompactionCandidateFileNum = 500;
Review Comment:
Suggest to change to `FileLimitPerCrossTask`
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #10069: [IOTDB-5972]Limit the number of all source files in compaction
Posted by "xingtanzjr (via GitHub)" <gi...@apache.org>.
xingtanzjr commented on code in PR #10069:
URL: https://github.com/apache/iotdb/pull/10069#discussion_r1222819374
##########
server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/exception/CompactionFileNumNotEnoughException.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * 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.iotdb.db.engine.compaction.execute.exception;
+
+public class CompactionFileNumNotEnoughException extends Exception {
Review Comment:
Suggest to rename to `CompactionFileCountExceededException`
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org