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/03/02 13:16:31 UTC
[GitHub] [ozone] bshashikant opened a new pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
bshashikant opened a new pull request #1981:
URL: https://github.com/apache/ozone/pull/1981
## What changes were proposed in this pull request?
Added SCM Ratis enable/disable switch
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4890
## How was this patch tested?
Existing unit tests should work with the patch on and off.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588063654
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
##########
@@ -85,25 +94,27 @@ public SCMHAManagerImpl(final ConfigurationSource conf,
*/
@Override
public void start() throws IOException {
- ratisServer.start();
- if (ratisServer.getDivision().getGroup().getPeers().isEmpty()) {
- // this is a bootstrapped node
- // It will first try to add itself to existing ring
- boolean success = HAUtils.addSCM(OzoneConfiguration.of(conf),
- new AddSCMRequest.Builder().setClusterId(scm.getClusterId())
- .setScmId(scm.getScmId()).setRatisAddr(
- scm.getSCMHANodeDetails().getLocalNodeDetails()
- // TODO : Should we use IP instead of hostname??
- .getRatisHostPortStr()).build(), scm.getSCMNodeId());
- if (!success) {
- throw new IOException("Adding SCM to existing HA group failed");
+ if (ratisServer != null) {
Review comment:
avoid too deep indent and un-necessary change.
How about
```
if (ratisServer == null) {
return
}
// original code
```
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588006734
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
##########
@@ -194,8 +192,7 @@ public void increaseRetryCountOfTransactionInDB(
// then set the retry value to -1, stop retrying, admins can
// analyze those blocks and purge them manually by SCMCli.
DeletedBlocksTransaction.Builder builder = block.toBuilder().setCount(-1);
- deletedTable.putWithBatch(
- transactionBuffer.getCurrentBatchOperation(), txID, builder.build());
+ transactionBuffer.addToBuffer(deletedTable, txID, builder.build());
Review comment:
there will be memory leak in `deletingTxIDs` and `skippingRetryTxIDs`, since `onFlush` won/t be called without ratis.
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/metadata/DBTransactionBuffer.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.hdds.scm.metadata;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+
+import java.io.Closeable;
+import java.io.IOException;
+
+/**
+ * DB transaction that abstracts the updates to the underlying datastore.
+ */
+public interface DBTransactionBuffer<KEY, VALUE> extends Closeable {
+
+ void addToBuffer(Table<KEY, VALUE> table, KEY key, VALUE value) throws
+ IOException;
+
+ void removeFromBuffer(Table<KEY, VALUE> table, KEY key) throws
+ IOException;
+
+ void close() throws IOException;
Review comment:
no need of this close method, exactly the same as that in `Closeable`
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
Agree. Can `MockSCMHADBTransactionBuffer` share the same logic `addToBuffer/removeFromBuffer` as `SCMDBTransactionBufferImpl`, and let its `flush` to throw exception ?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
##########
@@ -85,25 +94,27 @@ public SCMHAManagerImpl(final ConfigurationSource conf,
*/
@Override
public void start() throws IOException {
- ratisServer.start();
- if (ratisServer.getDivision().getGroup().getPeers().isEmpty()) {
- // this is a bootstrapped node
- // It will first try to add itself to existing ring
- boolean success = HAUtils.addSCM(OzoneConfiguration.of(conf),
- new AddSCMRequest.Builder().setClusterId(scm.getClusterId())
- .setScmId(scm.getScmId()).setRatisAddr(
- scm.getSCMHANodeDetails().getLocalNodeDetails()
- // TODO : Should we use IP instead of hostname??
- .getRatisHostPortStr()).build(), scm.getSCMNodeId());
- if (!success) {
- throw new IOException("Adding SCM to existing HA group failed");
+ if (ratisServer != null) {
Review comment:
avoid too deep indent.
How about
```
if (ratisServer == null) {
return
}
// original code
```
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAInvocationHandler.java
##########
@@ -50,16 +50,20 @@ public SCMHAInvocationHandler(final RequestType requestType,
this.requestType = requestType;
this.localHandler = localHandler;
this.ratisHandler = ratisHandler;
- ratisHandler.registerStateMachineHandler(requestType, localHandler);
+ if (ratisHandler != null) {
+ ratisHandler.registerStateMachineHandler(requestType, localHandler);
+ }
}
@Override
public Object invoke(final Object proxy, final Method method,
final Object[] args) throws Throwable {
try {
long startTime = Time.monotonicNow();
- final Object result = method.isAnnotationPresent(Replicate.class) ?
- invokeRatis(method, args) : invokeLocal(method, args);
+ final Object result =
Review comment:
Please add a Preconditions.checkNotNull for ratisHandler at invokeRatis too. Thanks!
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
##########
@@ -170,8 +169,7 @@ public void removeTransactionsFromDB(ArrayList<Long> txIDs)
throws IOException {
deletingTxIDs.addAll(txIDs);
for (Long txID : txIDs) {
- deletedTable.deleteWithBatch(
- transactionBuffer.getCurrentBatchOperation(), txID);
+ transactionBuffer.removeFromBuffer(deletedTable, txID);
Review comment:
Should not call `deletingTxIDs.addAll(txIDs);` if ratis is not enabled.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -68,12 +68,7 @@
import java.io.IOException;
import java.net.InetSocketAddress;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
+import java.util.*;
Review comment:
expand it ?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHADBTransactionBuffer.java
##########
@@ -20,22 +20,22 @@
import org.apache.hadoop.hdds.utils.db.BatchOperation;
import org.apache.hadoop.hdds.utils.db.DBStore;
import org.apache.hadoop.hdds.utils.db.RDBBatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.ratis.statemachine.SnapshotInfo;
import java.io.IOException;
-public class MockDBTransactionBuffer implements DBTransactionBuffer {
+public class MockSCMHADBTransactionBuffer implements SCMHADBTransactionBuffer {
private DBStore dbStore;
private BatchOperation currentBatchOperation;
- public MockDBTransactionBuffer() {
+ public MockSCMHADBTransactionBuffer() {
}
- public MockDBTransactionBuffer(DBStore store) {
+ public MockSCMHADBTransactionBuffer(DBStore store) {
this.dbStore = store;
}
- @Override
public BatchOperation getCurrentBatchOperation() {
Review comment:
change getCurrentBatchOperation to private.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestUtils.java
##########
@@ -476,7 +476,7 @@ public static void quasiCloseContainer(ContainerManagerV2 containerManager,
public static StorageContainerManager getScmSimple(OzoneConfiguration conf)
throws IOException, AuthenticationException {
SCMConfigurator configurator = new SCMConfigurator();
- conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
+ // conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
Review comment:
I guess, this is the flag to enable ratis SCM in test ? Better add a comment, since it is easy to overlook.
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/metadata/DBTransactionBuffer.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.hdds.scm.metadata;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+
+import java.io.Closeable;
+import java.io.IOException;
+
+/**
+ * DB transaction that abstracts the updates to the underlying datastore.
+ */
+public interface DBTransactionBuffer<KEY, VALUE> extends Closeable {
+
+ void addToBuffer(Table<KEY, VALUE> table, KEY key, VALUE value) throws
+ IOException;
+
+ void removeFromBuffer(Table<KEY, VALUE> table, KEY key) throws
Review comment:
generic method may be better than generic class here. We can fix it later if it is too complicated.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -454,10 +454,11 @@ private void initializeSystemManagers(OzoneConfiguration conf,
if (configurator.getScmContext() != null) {
scmContext = configurator.getScmContext();
} else {
+ long term = SCMHAUtils.isSCMHAEnabled(conf) ? 0 : SCMContext.INVALID_TERM;
Review comment:
Good !
Better add a comment here.
```
// When term equals SCMContext.INVALID_TERM, the isLeader() and getTermOfLeader() will always pass.
```
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588072095
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -163,26 +165,24 @@ public void tearDown() throws Exception {
private void addTransactions(Map<Long, List<Long>> containerBlocksMap)
throws IOException {
- dbTransactionBuffer.getCurrentBatchOperation();
+ //SCMHADBTransactionBuffer.getCurrentBatchOperation();
Review comment:
remove it ?
----------------------------------------------------------------
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 pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#issuecomment-791952958
Thanks @bshashikant for the work!
One more thing about recon, `MockSCMHAManager` and `SCMHADBTransactionBuffer` is used by `ReconStorageContainerManagerFacade`, as
```
this.scmhaManager = MockSCMHAManager.getInstance(
true, new MockSCMHADBTransactionBuffer(dbStore));
```
Need replace `MockSCMHADBTransactionBuffer ` with `SCMDBTransactionBufferImpl`, otherwise recon will OOM, since flush is called.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588072848
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHADBTransactionBuffer.java
##########
@@ -20,22 +20,22 @@
import org.apache.hadoop.hdds.utils.db.BatchOperation;
import org.apache.hadoop.hdds.utils.db.DBStore;
import org.apache.hadoop.hdds.utils.db.RDBBatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.ratis.statemachine.SnapshotInfo;
import java.io.IOException;
-public class MockDBTransactionBuffer implements DBTransactionBuffer {
+public class MockSCMHADBTransactionBuffer implements SCMHADBTransactionBuffer {
private DBStore dbStore;
private BatchOperation currentBatchOperation;
- public MockDBTransactionBuffer() {
+ public MockSCMHADBTransactionBuffer() {
}
- public MockDBTransactionBuffer(DBStore store) {
+ public MockSCMHADBTransactionBuffer(DBStore store) {
this.dbStore = store;
}
- @Override
public BatchOperation getCurrentBatchOperation() {
Review comment:
+1. I guess also need to remove it from the interface?
----------------------------------------------------------------
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] bshashikant commented on a change in pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588207146
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/metadata/DBTransactionBuffer.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.hdds.scm.metadata;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+
+import java.io.Closeable;
+import java.io.IOException;
+
+/**
+ * DB transaction that abstracts the updates to the underlying datastore.
+ */
+public interface DBTransactionBuffer<KEY, VALUE> extends Closeable {
+
+ void addToBuffer(Table<KEY, VALUE> table, KEY key, VALUE value) throws
+ IOException;
+
+ void removeFromBuffer(Table<KEY, VALUE> table, KEY key) throws
Review comment:
Will address it in a different PR.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588841438
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -202,8 +203,8 @@ public Boolean allocateBatch(String sequenceIdName,
}
try {
- sequenceIdTable.putWithBatch(transactionBuffer
- .getCurrentBatchOperation(), sequenceIdName, newLastId);
+ transactionBuffer
+ .addToBuffer(sequenceIdTable, sequenceIdName, newLastId);
Review comment:
You need also remove `Preconditions.checkNotNull(ratisServer);` in line 248
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588842185
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -202,8 +203,8 @@ public Boolean allocateBatch(String sequenceIdName,
}
try {
- sequenceIdTable.putWithBatch(transactionBuffer
- .getCurrentBatchOperation(), sequenceIdName, newLastId);
+ transactionBuffer
+ .addToBuffer(sequenceIdTable, sequenceIdName, newLastId);
Review comment:
My fault. It should be fine. non-ratis SCM will not go to there.
----------------------------------------------------------------
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] bshashikant commented on pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#issuecomment-792441027
Thanks @GlenGeng for the review. I have committed this.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r587728174
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
Is it possible to implement "flush on every write" in `MockSCMHADBTransactionBuffer` so in tests we no longer need to call flush()?
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588065406
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -454,10 +454,11 @@ private void initializeSystemManagers(OzoneConfiguration conf,
if (configurator.getScmContext() != null) {
scmContext = configurator.getScmContext();
} else {
+ long term = SCMHAUtils.isSCMHAEnabled(conf) ? 0 : SCMContext.INVALID_TERM;
Review comment:
Good !
Better add a comment here.
```
// When term equals SCMContext.INVALID_TERM, the isLeader() check and getTermOfLeader() will always pass.
```
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588072848
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHADBTransactionBuffer.java
##########
@@ -20,22 +20,22 @@
import org.apache.hadoop.hdds.utils.db.BatchOperation;
import org.apache.hadoop.hdds.utils.db.DBStore;
import org.apache.hadoop.hdds.utils.db.RDBBatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.ratis.statemachine.SnapshotInfo;
import java.io.IOException;
-public class MockDBTransactionBuffer implements DBTransactionBuffer {
+public class MockSCMHADBTransactionBuffer implements SCMHADBTransactionBuffer {
private DBStore dbStore;
private BatchOperation currentBatchOperation;
- public MockDBTransactionBuffer() {
+ public MockSCMHADBTransactionBuffer() {
}
- public MockDBTransactionBuffer(DBStore store) {
+ public MockSCMHADBTransactionBuffer(DBStore store) {
this.dbStore = store;
}
- @Override
public BatchOperation getCurrentBatchOperation() {
Review comment:
+1
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588061172
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAInvocationHandler.java
##########
@@ -50,16 +50,20 @@ public SCMHAInvocationHandler(final RequestType requestType,
this.requestType = requestType;
this.localHandler = localHandler;
this.ratisHandler = ratisHandler;
- ratisHandler.registerStateMachineHandler(requestType, localHandler);
+ if (ratisHandler != null) {
+ ratisHandler.registerStateMachineHandler(requestType, localHandler);
+ }
}
@Override
public Object invoke(final Object proxy, final Method method,
final Object[] args) throws Throwable {
try {
long startTime = Time.monotonicNow();
- final Object result = method.isAnnotationPresent(Replicate.class) ?
- invokeRatis(method, args) : invokeLocal(method, args);
+ final Object result =
Review comment:
Please add a Preconditions.checkNotNull for ratisHandler inside invokeRatis too. Thanks!
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588213021
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
##########
@@ -193,14 +198,19 @@ public void increaseRetryCountOfTransactionInDB(
// analyze those blocks and purge them manually by SCMCli.
DeletedBlocksTransaction.Builder builder = block.toBuilder().setCount(-1);
transactionBuffer.addToBuffer(deletedTable, txID, builder.build());
- skippingRetryTxIDs.add(txID);
+ if (skippingRetryTxIDs != null) {
+ skippingRetryTxIDs.add(txID);
+ }
}
}
public void onFlush() {
- deletingTxIDs.clear();
- skippingRetryTxIDs.clear();
+ if (deletingTxIDs != null) {
Review comment:
Better to be
```
// onFlush() can be invoked only when ratis is enabled.
Preconditions.checkNotNull(deletingTxIDs);
Preconditions.checkNotNull(skippingRetryTxIDs);
deletingTxIDs.clear();
skippingRetryTxIDs.clear();
```
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
##########
@@ -89,8 +92,8 @@ private void findNext() {
throw new IllegalStateException("");
}
- if (!deletingTxIDs.contains(txID) &&
- !skippingRetryTxIDs.contains(txID)) {
+ if (deletingTxIDs != null && !deletingTxIDs.contains(txID) &&
Review comment:
```
(deletingTxIDs == null || !deletingTxIDs.contains(txID)) &&
(skippingRetryTxIDs == null || !skippingRetryTxIDs.contains(txID))
```
----------------------------------------------------------------
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] bshashikant merged pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
bshashikant merged pull request #1981:
URL: https://github.com/apache/ozone/pull/1981
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r586238055
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java
##########
@@ -204,32 +199,29 @@ private Message process(final SCMRatisRequest request)
}
}
- @Override
- public void stop() {
+ @Override public void stop() {
}
- @Override
- public RaftServer.Division getDivision() {
+ @Override public RaftServer.Division getDivision() {
return null;
}
- @Override
- public List<String> getRatisRoles() {
- return Arrays.asList(
- "180.3.14.5:9865",
- "180.3.14.21:9865",
- "180.3.14.145:9865");
+ @Override public List<String> getRatisRoles() {
Review comment:
The code style is weird, why put override and function name in the same line ?
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/metadata/SCMDBTransactionBufferImpl.java
##########
@@ -0,0 +1,34 @@
+
+
+package org.apache.hadoop.hdds.scm.metadata;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+
+import java.io.IOException;
+
+/**
+ * Default implementation for DBTransactionBuffer for SCM without Ratis.
+ */
+public class SCMDBTransactionBufferImpl implements DBTransactionBuffer {
+
+ public SCMDBTransactionBufferImpl() {
+
+ }
+
+ @Override
+ public void addToBuffer(Table table, Object key, Object value)
+ throws IOException {
+ table.put(key, value);
Review comment:
Quite straightforward, I like it.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588841020
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
One tricky thing is that, recon uses `MockSCMHAManager` and `MockSCMHADBTransactionBuffer`, and recon needs actually a in-memory SCM.
Since Recon will not use ratis, `flush` is not called, thus there will finally be OOM for recon.
```
Ctor of ReconStorageContainerManagerFacade
this.scmhaManager = MockSCMHAManager.getInstance(
true, new MockSCMHADBTransactionBuffer(dbStore));
```
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r587728632
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
Or, even better, `MockSCMHADBTransactionBuffer` just pass through the buffer and write to table directly?
----------------------------------------------------------------
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] bshashikant commented on a change in pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588067780
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
I think, MockInstances should mimic every functional behaviour of the class. I would prefer to keep it as it is for now. We should be able to test the batch behaviour of SCMHADBTransaction behaviour with this.
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r587726530
##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/metadata/SCMDBTransactionBufferImpl.java
##########
@@ -0,0 +1,34 @@
+
+
+package org.apache.hadoop.hdds.scm.metadata;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+
+import java.io.IOException;
+
+/**
+ * Default implementation for DBTransactionBuffer for SCM without Ratis.
+ */
+public class SCMDBTransactionBufferImpl implements DBTransactionBuffer {
+
+ public SCMDBTransactionBufferImpl() {
+
+ }
+
+ @Override
+ public void addToBuffer(Table table, Object key, Object value)
+ throws IOException {
+ table.put(key, value);
Review comment:
Actually very nice! Thanks!
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588072626
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
I see. Sure it sounds good.
----------------------------------------------------------------
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] bshashikant commented on a change in pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588225580
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogStateManagerImpl.java
##########
@@ -89,8 +92,8 @@ private void findNext() {
throw new IllegalStateException("");
}
- if (!deletingTxIDs.contains(txID) &&
- !skippingRetryTxIDs.contains(txID)) {
+ if (deletingTxIDs != null && !deletingTxIDs.contains(txID) &&
Review comment:
Thanks for pointing out. Will address in the next 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] GlenGeng commented on a change in pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588841020
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
##########
@@ -85,23 +86,24 @@
private ContainerManagerV2 containerManager;
private StorageContainerManager scm;
private List<DatanodeDetails> dnList;
- private DBTransactionBuffer dbTransactionBuffer;
+ private SCMHADBTransactionBuffer SCMHADBTransactionBuffer;
@Before
public void setup() throws Exception {
testDir = GenericTestUtils.getTestDir(
TestDeletedBlockLog.class.getSimpleName());
conf = new OzoneConfiguration();
+ conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true);
conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20);
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
scm = TestUtils.getScm(conf);
containerManager = Mockito.mock(ContainerManagerV2.class);
- dbTransactionBuffer =
- new MockDBTransactionBuffer(scm.getScmMetadataStore().getStore());
+ SCMHADBTransactionBuffer =
+ new MockSCMHADBTransactionBuffer(scm.getScmMetadataStore().getStore());
Review comment:
One tricky thing is that, recon uses `MockSCMHAManager` and `MockSCMHADBTransactionBuffer`.
Since Recon will not use ratis, `flush` is not called, thus there will finally be OOM for recon.
```
Ctor of ReconStorageContainerManagerFacade
this.scmhaManager = MockSCMHAManager.getInstance(
true, new MockSCMHADBTransactionBuffer(dbStore));
```
----------------------------------------------------------------
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 #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#discussion_r588222369
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -175,9 +175,9 @@ public SCMRatisResponse submitRequest(SCMRatisRequest request)
.build();
final RaftClientReply raftClientReply =
server.submitClientRequestAsync(raftClientRequest).get();
- if (LOG.isDebugEnabled()) {
- LOG.debug("request {} Reply {}", raftClientRequest, raftClientReply);
- }
+ // if (LOG.isDebugEnabled()) {
Review comment:
this should be a temporary change ?
----------------------------------------------------------------
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 pull request #1981: HDDS-4890. SCM Ratis enable/disable switch
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #1981:
URL: https://github.com/apache/ozone/pull/1981#issuecomment-792423946
+1. Thanks @bshashikant for the contribution.
----------------------------------------------------------------
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