You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/01/05 01:46:59 UTC
[GitHub] [ozone] amaliujia opened a new pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
amaliujia opened a new pull request #1756:
URL: https://github.com/apache/ozone/pull/1756
## What changes were proposed in this pull request?
Right now Ratis snapshot auto trigger threshold is a fixed number. We should load it from the HA configuration.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4643
## How was this patch tested?
N/A
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551704355
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAConfiguration.java
##########
@@ -114,6 +114,14 @@
)
private int raftLogPurgeGap = 1000000;
+ @Config(key = "ratis.snapshot.threshold",
+ type = ConfigType.LONG,
+ defaultValue = "100000L",
+ tags = {SCM, OZONE, HA, RATIS},
+ description = "The threshold to trigger a Ratis taking snapshot " +
+ "operation")
Review comment:
seems not a typo?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] GlenGeng commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551701803
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAConfiguration.java
##########
@@ -114,6 +114,14 @@
)
private int raftLogPurgeGap = 1000000;
+ @Config(key = "ratis.snapshot.threshold",
+ type = ConfigType.LONG,
+ defaultValue = "100000L",
+ tags = {SCM, OZONE, HA, RATIS},
+ description = "The threshold to trigger a Ratis taking snapshot " +
+ "operation")
Review comment:
operation -> operation for SCM.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAConfiguration.java
##########
@@ -114,6 +114,14 @@
)
private int raftLogPurgeGap = 1000000;
+ @Config(key = "ratis.snapshot.threshold",
+ type = ConfigType.LONG,
+ defaultValue = "100000L",
+ tags = {SCM, OZONE, HA, RATIS},
+ description = "The threshold to trigger a Ratis taking snapshot " +
+ "operation")
+ private long ratisSnapshotThreshold = 100000L;
Review comment:
100000L might be too large for SCM, since the WriteBatch needs hold them in memory and the flush will block the apply of transactions. How about 500 or 1000 ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] GlenGeng commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551714814
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAConfiguration.java
##########
@@ -114,6 +114,14 @@
)
private int raftLogPurgeGap = 1000000;
+ @Config(key = "ratis.snapshot.threshold",
+ type = ConfigType.LONG,
+ defaultValue = "100000L",
+ tags = {SCM, OZONE, HA, RATIS},
+ description = "The threshold to trigger a Ratis taking snapshot " +
+ "operation")
Review comment:
Not a typo, the description indicates that this config is for SCM.
"The threshold to trigger a Ratis taking snapshot operation" -> "The threshold to trigger a Ratis taking snapshot operation for SCM StateMachine"
BTW, please change the `defaultValue` too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551670693
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/RatisUtil.java
##########
@@ -149,7 +149,8 @@ private static void setRaftRetryCacheProperties(
private static void setRaftSnapshotProperties(
final RaftProperties properties, final SCMHAConfiguration conf) {
Snapshot.setAutoTriggerEnabled(properties, true);
- Snapshot.setAutoTriggerThreshold(properties, 400000);
+ Snapshot.setAutoTriggerThreshold(properties,
+ conf.getRatisSnapshotThreshold());
Review comment:
This is useful for both writing unit tests for snapshot functionality and allow users to set their own threshold
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551671227
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestReplicationAnnotation.java
##########
@@ -88,8 +88,7 @@ public void testReplicateAnnotationBasic() throws Throwable {
try {
proxy.addContainer(HddsProtos.ContainerInfoProto.getDefaultInstance());
- // Should have seen a IOException.
- Assert.fail();
+ Assert.fail("Cannot reach here: should have seen a IOException");
Review comment:
Have this line of change due to https://github.com/apache/ozone/pull/1650#discussion_r551137844
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] runzhiwang commented on pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#issuecomment-755008624
@amaliujia Thanks the patch. @GlenGeng Thanks for review. I have merged this patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#issuecomment-754332214
@runzhiwang @GlenGeng @ChenSammi @nandakumar131
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on a change in pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#discussion_r551704328
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAConfiguration.java
##########
@@ -114,6 +114,14 @@
)
private int raftLogPurgeGap = 1000000;
+ @Config(key = "ratis.snapshot.threshold",
+ type = ConfigType.LONG,
+ defaultValue = "100000L",
+ tags = {SCM, OZONE, HA, RATIS},
+ description = "The threshold to trigger a Ratis taking snapshot " +
+ "operation")
+ private long ratisSnapshotThreshold = 100000L;
Review comment:
make senses. Set as 1000 for now. This is a configurable value anyway.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] runzhiwang merged pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
runzhiwang merged pull request #1756:
URL: https://github.com/apache/ozone/pull/1756
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on pull request #1756: HDDS-4643. Ratis Snapshot should be loaded from the config
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1756:
URL: https://github.com/apache/ozone/pull/1756#issuecomment-755005721
@runzhiwang CI has passed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org