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/05 02:16:37 UTC
[GitHub] [ozone] GlenGeng opened a new pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
GlenGeng opened a new pull request #1980:
URL: https://github.com/apache/ozone/pull/1980
## What changes were proposed in this pull request?
Monotonically Increased ID Supporting Switch Between Non-Ratis SCM and Ratis-Based SCM.
1. SCM allocates ids in a batch way. It maintains two fields: lastId and nextId. It saves the lastId in rocksDB. The initial value of nextId is firstId.
2. When getNextId() is called, if nextId is less than lastId, SCM just returns nextId and increases it, otherwise it calls allocateBatch(expectedLastId, newLastId) to allocate a batch of ids, then serves the request.
3. In allocateBatch(expectedLastId, newLastId) , it works in CAS way:
it reads lastId from rocksDB.
if lastId equals expectedLastId, it saves newLastId into rocksDB, returns success, otherwise it rejects the allocation request.
It also provides a getLastId() to read lastId from rocksDB. The allocation works in a loop way:
See https://docs.google.com/document/d/13qSwLuDBw7mytJtF9h2oauuLocFfr-XJSBdTfc1YvxM/edit?usp=sharing
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4651
## How was this patch tested?
CI
----------------------------------------------------------------
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 closed pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng closed pull request #1980:
URL: https://github.com/apache/ozone/pull/1980
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789381266
@mukul1987 @bshashikant @amaliujia Please take look!
Monotonically Increased ID Supporting Switch Between Non-Ratis SCM and Ratis-Based SCM.
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#discussion_r586120243
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSequenceIDGen.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore;
+import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStoreImpl;
+import org.apache.hadoop.ozone.container.common.SCMTestUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+
+public class TestSequenceIDGen {
+ @Test
+ public void testSequenceIDGenUponNonRatis() throws Exception {
Review comment:
Agree. Will create a jira this. It is a good newbie job.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/BooleanCodec.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.ha.io;
+
+import com.google.protobuf.ByteString;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class BooleanCodec implements Codec {
Review comment:
We have this method, thus we need this `BooleanCodec`
```
@Replicate
Boolean allocateBatch(Long expectedLastId, Long newLastId);
```
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#discussion_r586279853
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGen.java
##########
@@ -0,0 +1,284 @@
+/*
Review comment:
can we change the class name SequenceIdGen.java ---> SequenceIdGenerator.java
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGen.java
##########
@@ -0,0 +1,284 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.metadata.Replicate;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.reflect.Proxy;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType.SEQUENCE_ID;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT;
+import static org.apache.hadoop.ozone.OzoneConsts.SEQUENCE_ID_KEY;
+
+/**
+ * After SCM starts, set lastId = 0, nextId = lastId + 1.
+ * The first getNextId() call triggers SCM to load lastId from rocksDB,
+ * and allocate a new batch.
+ *
+ * In order to maintain monotonicity, for Ratis based SequenceIdGen,
+ * when becoming leader, SCM invalidates un-exhausted id batch by setting
+ * nextId = lastId + 1, so that a new leader will reload lastId from
+ * rocksDB and allocate a new batch when receiving its first getNextId() call.
+ */
+public class SequenceIdGen {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SequenceIdGen.class);
+ private static final long INVALID_SEQUENCE_ID = 0;
+
+ private long lastId = INVALID_SEQUENCE_ID;
+ private long nextId = lastId + 1;
+
+ private final Lock lock;
+ private final long batchSize;
+ private final StateManager stateManager;
+
+ /**
+ * @param conf : conf
+ * @param scmhaManager : null if non-Ratis based
+ * @param sequenceIdTable : sequenceIdTable
+ */
+ public SequenceIdGen(ConfigurationSource conf, SCMHAManager scmhaManager,
+ Table<String, Long> sequenceIdTable) {
+ this.lock = new ReentrantLock();
+ this.batchSize = conf.getInt(OZONE_SCM_SEQUENCE_ID_BATCH_SIZE,
+ OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT);
+
+ if (SCMHAUtils.isSCMHAEnabled(conf)) {
+ this.stateManager = new StateManagerRatisImpl.Builder()
+ .setRatisServer(scmhaManager.getRatisServer())
+ .setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setSequenceIdTable(sequenceIdTable)
+ .build();
+ } else {
+ this.stateManager = new StateManagerNonRatisImpl(sequenceIdTable);
+ }
+ }
+
+ /**
+ * @return next distributed sequence id.
+ */
+ public long getNextId() {
+ lock.lock();
+ try {
+ if (nextId <= lastId) {
+ return nextId++;
+ }
+
+ Preconditions.checkArgument(nextId == lastId + 1);
+ while (true) {
+ Long prevLastId = lastId;
+ nextId = prevLastId + 1;
+ lastId += batchSize;
+
+ if (stateManager.allocateBatch(prevLastId, lastId)) {
+ LOG.info("Allocate a batch, change lastId from {} to {}.",
+ prevLastId, lastId);
+ break;
+ }
+
+ // reload lastId from RocksDB.
+ lastId = stateManager.getLastId();
+ }
+
+ Preconditions.checkArgument(nextId <= lastId);
+ return nextId++;
+
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Invalidate any un-exhausted batch, next getNextId() call will
+ * allocate a new batch.
+ */
+ public void invalidateBatch() {
+ lock.lock();
+ try {
+ nextId = lastId + 1;
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Maintain SequenceIdTable in RocksDB.
+ */
+ interface StateManager {
+ /**
+ * Compare And Swap lastId saved in db from expectedLastId to newLastId.
+ * If based on Ratis, it will submit a raft client request.
+ *
+ * @param expectedLastId : the expected lastId saved in db
+ * @param newLastId : the new lastId to save in db
+ * @return : result of the C.A.S.
+ */
+ @Replicate
+ Boolean allocateBatch(Long expectedLastId, Long newLastId);
+
+ /**
+ * @return lastId saved in db
+ */
+ Long getLastId();
+ }
+
+ /**
+ * Ratis based StateMachine, db operations are queued in
+ * DBTransactionBuffer until a snapshot is taken.
+ */
+ static final class StateManagerRatisImpl implements StateManager {
Review comment:
StateManagerRatisImpl -> StateManagerHAImpl
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGen.java
##########
@@ -0,0 +1,284 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.metadata.Replicate;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.reflect.Proxy;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType.SEQUENCE_ID;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT;
+import static org.apache.hadoop.ozone.OzoneConsts.SEQUENCE_ID_KEY;
+
+/**
+ * After SCM starts, set lastId = 0, nextId = lastId + 1.
+ * The first getNextId() call triggers SCM to load lastId from rocksDB,
+ * and allocate a new batch.
+ *
+ * In order to maintain monotonicity, for Ratis based SequenceIdGen,
+ * when becoming leader, SCM invalidates un-exhausted id batch by setting
+ * nextId = lastId + 1, so that a new leader will reload lastId from
+ * rocksDB and allocate a new batch when receiving its first getNextId() call.
+ */
+public class SequenceIdGen {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SequenceIdGen.class);
+ private static final long INVALID_SEQUENCE_ID = 0;
+
+ private long lastId = INVALID_SEQUENCE_ID;
+ private long nextId = lastId + 1;
+
+ private final Lock lock;
+ private final long batchSize;
+ private final StateManager stateManager;
+
+ /**
+ * @param conf : conf
+ * @param scmhaManager : null if non-Ratis based
+ * @param sequenceIdTable : sequenceIdTable
+ */
+ public SequenceIdGen(ConfigurationSource conf, SCMHAManager scmhaManager,
+ Table<String, Long> sequenceIdTable) {
+ this.lock = new ReentrantLock();
+ this.batchSize = conf.getInt(OZONE_SCM_SEQUENCE_ID_BATCH_SIZE,
+ OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT);
+
+ if (SCMHAUtils.isSCMHAEnabled(conf)) {
+ this.stateManager = new StateManagerRatisImpl.Builder()
+ .setRatisServer(scmhaManager.getRatisServer())
+ .setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setSequenceIdTable(sequenceIdTable)
+ .build();
+ } else {
+ this.stateManager = new StateManagerNonRatisImpl(sequenceIdTable);
+ }
+ }
+
+ /**
+ * @return next distributed sequence id.
+ */
+ public long getNextId() {
+ lock.lock();
+ try {
+ if (nextId <= lastId) {
+ return nextId++;
+ }
+
+ Preconditions.checkArgument(nextId == lastId + 1);
+ while (true) {
+ Long prevLastId = lastId;
+ nextId = prevLastId + 1;
+ lastId += batchSize;
+
+ if (stateManager.allocateBatch(prevLastId, lastId)) {
+ LOG.info("Allocate a batch, change lastId from {} to {}.",
+ prevLastId, lastId);
+ break;
+ }
+
+ // reload lastId from RocksDB.
+ lastId = stateManager.getLastId();
+ }
+
+ Preconditions.checkArgument(nextId <= lastId);
+ return nextId++;
+
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Invalidate any un-exhausted batch, next getNextId() call will
+ * allocate a new batch.
+ */
+ public void invalidateBatch() {
+ lock.lock();
+ try {
+ nextId = lastId + 1;
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Maintain SequenceIdTable in RocksDB.
+ */
+ interface StateManager {
+ /**
+ * Compare And Swap lastId saved in db from expectedLastId to newLastId.
+ * If based on Ratis, it will submit a raft client request.
+ *
+ * @param expectedLastId : the expected lastId saved in db
+ * @param newLastId : the new lastId to save in db
+ * @return : result of the C.A.S.
+ */
+ @Replicate
+ Boolean allocateBatch(Long expectedLastId, Long newLastId);
+
+ /**
+ * @return lastId saved in db
+ */
+ Long getLastId();
+ }
+
+ /**
+ * Ratis based StateMachine, db operations are queued in
+ * DBTransactionBuffer until a snapshot is taken.
+ */
+ static final class StateManagerRatisImpl implements StateManager {
+ private final Table<String, Long> sequenceIdTable;
+ private final DBTransactionBuffer transactionBuffer;
+ private volatile Long lastId;
+
+ private StateManagerRatisImpl(Table<String, Long> sequenceIdTable,
+ DBTransactionBuffer trxBuffer) {
+ this.sequenceIdTable = sequenceIdTable;
+ this.transactionBuffer = trxBuffer;
+
+ try {
+ lastId = this.sequenceIdTable.get(SEQUENCE_ID_KEY);
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to get lastId from db", ioe);
+ }
+ if (lastId == null) {
+ lastId = INVALID_SEQUENCE_ID;
+ }
+ LOG.info("Init Ratis based SequenceIdGen, lastId is {}.", lastId);
+ }
+
+ @Override
+ public Boolean allocateBatch(Long expectedLastId, Long newLastId) {
+ if (!lastId.equals(expectedLastId)) {
+ LOG.warn("Failed to allocate a batch, expected lastId is {}," +
+ " actual lastId is {}.", expectedLastId, lastId);
+ return false;
+ }
+
+ try {
+ sequenceIdTable.putWithBatch(transactionBuffer
+ .getCurrentBatchOperation(), SEQUENCE_ID_KEY, newLastId);
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to put lastId to Batch", ioe);
+ }
+
+ lastId = newLastId;
+ return true;
+ }
+
+ @Override
+ public Long getLastId() {
+ return lastId;
+ }
+
+ /**
+ * Builder for Ratis based StateManager.
+ */
+ public static class Builder {
+ private Table<String, Long> table;
+ private DBTransactionBuffer buffer;
+ private SCMRatisServer ratisServer;
+
+ public Builder setRatisServer(final SCMRatisServer scmRatisServer) {
+ this.ratisServer = scmRatisServer;
+ return this;
+ }
+
+ public Builder setSequenceIdTable(
+ final Table<String, Long> sequenceIdTable) {
+ table = sequenceIdTable;
+ return this;
+ }
+
+ public Builder setDBTransactionBuffer(DBTransactionBuffer trxBuffer) {
+ buffer = trxBuffer;
+ return this;
+ }
+
+ public StateManager build() {
+ Preconditions.checkNotNull(table);
+ Preconditions.checkNotNull(buffer);
+ Preconditions.checkNotNull(ratisServer);
+
+ final StateManager impl = new StateManagerRatisImpl(table, buffer);
+ final SCMHAInvocationHandler invocationHandler
+ = new SCMHAInvocationHandler(SEQUENCE_ID, impl, ratisServer);
+
+ return (StateManager) Proxy.newProxyInstance(
+ SCMHAInvocationHandler.class.getClassLoader(),
+ new Class<?>[]{StateManager.class},
+ invocationHandler);
+ }
+ }
+ }
+
+ /**
+ * non-Ratis based StateMachine, db operations directly go to RocksDB.
+ */
+ static final class StateManagerNonRatisImpl implements StateManager {
+ private final Table<String, Long> sequenceIdTable;
Review comment:
StateManagerNonRatisImpl -> StateManagerImpl
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGen.java
##########
@@ -0,0 +1,284 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.metadata.Replicate;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.reflect.Proxy;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType.SEQUENCE_ID;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT;
+import static org.apache.hadoop.ozone.OzoneConsts.SEQUENCE_ID_KEY;
+
+/**
+ * After SCM starts, set lastId = 0, nextId = lastId + 1.
+ * The first getNextId() call triggers SCM to load lastId from rocksDB,
+ * and allocate a new batch.
+ *
+ * In order to maintain monotonicity, for Ratis based SequenceIdGen,
+ * when becoming leader, SCM invalidates un-exhausted id batch by setting
+ * nextId = lastId + 1, so that a new leader will reload lastId from
+ * rocksDB and allocate a new batch when receiving its first getNextId() call.
+ */
+public class SequenceIdGen {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SequenceIdGen.class);
+ private static final long INVALID_SEQUENCE_ID = 0;
+
+ private long lastId = INVALID_SEQUENCE_ID;
+ private long nextId = lastId + 1;
+
+ private final Lock lock;
+ private final long batchSize;
+ private final StateManager stateManager;
+
+ /**
+ * @param conf : conf
+ * @param scmhaManager : null if non-Ratis based
+ * @param sequenceIdTable : sequenceIdTable
+ */
+ public SequenceIdGen(ConfigurationSource conf, SCMHAManager scmhaManager,
+ Table<String, Long> sequenceIdTable) {
+ this.lock = new ReentrantLock();
+ this.batchSize = conf.getInt(OZONE_SCM_SEQUENCE_ID_BATCH_SIZE,
+ OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT);
+
+ if (SCMHAUtils.isSCMHAEnabled(conf)) {
+ this.stateManager = new StateManagerRatisImpl.Builder()
+ .setRatisServer(scmhaManager.getRatisServer())
+ .setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setSequenceIdTable(sequenceIdTable)
+ .build();
+ } else {
+ this.stateManager = new StateManagerNonRatisImpl(sequenceIdTable);
+ }
+ }
+
+ /**
+ * @return next distributed sequence id.
+ */
+ public long getNextId() {
+ lock.lock();
+ try {
+ if (nextId <= lastId) {
+ return nextId++;
+ }
+
+ Preconditions.checkArgument(nextId == lastId + 1);
+ while (true) {
+ Long prevLastId = lastId;
+ nextId = prevLastId + 1;
+ lastId += batchSize;
+
+ if (stateManager.allocateBatch(prevLastId, lastId)) {
+ LOG.info("Allocate a batch, change lastId from {} to {}.",
+ prevLastId, lastId);
+ break;
+ }
+
+ // reload lastId from RocksDB.
+ lastId = stateManager.getLastId();
+ }
+
+ Preconditions.checkArgument(nextId <= lastId);
+ return nextId++;
+
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Invalidate any un-exhausted batch, next getNextId() call will
+ * allocate a new batch.
+ */
+ public void invalidateBatch() {
+ lock.lock();
+ try {
+ nextId = lastId + 1;
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Maintain SequenceIdTable in RocksDB.
+ */
+ interface StateManager {
+ /**
+ * Compare And Swap lastId saved in db from expectedLastId to newLastId.
+ * If based on Ratis, it will submit a raft client request.
+ *
+ * @param expectedLastId : the expected lastId saved in db
+ * @param newLastId : the new lastId to save in db
+ * @return : result of the C.A.S.
+ */
+ @Replicate
+ Boolean allocateBatch(Long expectedLastId, Long newLastId);
+
+ /**
+ * @return lastId saved in db
+ */
+ Long getLastId();
+ }
+
+ /**
+ * Ratis based StateMachine, db operations are queued in
+ * DBTransactionBuffer until a snapshot is taken.
+ */
+ static final class StateManagerRatisImpl implements StateManager {
+ private final Table<String, Long> sequenceIdTable;
+ private final DBTransactionBuffer transactionBuffer;
+ private volatile Long lastId;
+
+ private StateManagerRatisImpl(Table<String, Long> sequenceIdTable,
+ DBTransactionBuffer trxBuffer) {
+ this.sequenceIdTable = sequenceIdTable;
+ this.transactionBuffer = trxBuffer;
+
+ try {
+ lastId = this.sequenceIdTable.get(SEQUENCE_ID_KEY);
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to get lastId from db", ioe);
+ }
+ if (lastId == null) {
+ lastId = INVALID_SEQUENCE_ID;
+ }
+ LOG.info("Init Ratis based SequenceIdGen, lastId is {}.", lastId);
Review comment:
The log message needs to be changed accordingly.
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789703216
> > > Do we need to handle overflow for "long" used here for lastId?
> >
> >
> > Thanks for the advice, I've add the check.
> > > With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
> >
> >
> > Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
>
> What i meant was, containerIds are not sequentially increasing which is a departure from the current behaviour which is strictly sequentially increasing containerIds. and we are using the same seqID generator for both Container as well as block.
> This may become the reason for scale limitation.
>
> Instead of that, why can't we use lastAppliedIndex as base for containerIds? Sequence Ids can be used for block Ids and can be rest to zero once all the containers are closed.
>
> > > We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
> >
> >
> > We have a discussion in the design doc:
> > > need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
> > > localId : the initial value should be UniqueId() + 1
> > > containerID: scan the container db, figure the largest containerID ever used.
> > > delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
> >
> >
> > So, we can simply use `UniqueId() + 1` as `INVALID_SEQUENCE_ID`, since containerID and delTx are starting from 1, practically they can not be larger than `UniqueId() + 1`.
>
> Are u going to address these changes in subsequent PR as this will be required for branch merge?
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#discussion_r588076507
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -0,0 +1,308 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.metadata.Replicate;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType.SEQUENCE_ID;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT;
+
+/**
+ * After SCM starts, set lastId = 0, nextId = lastId + 1.
+ * The first getNextId() call triggers SCM to load lastId from rocksDB,
+ * and allocate a new batch.
+ *
+ * In order to maintain monotonicity, for Ratis based SequenceIdGen,
+ * when becoming leader, SCM invalidates un-exhausted id batch by setting
+ * nextId = lastId + 1, so that a new leader will reload lastId from
+ * rocksDB and allocate a new batch when receiving its first getNextId() call.
+ */
+public class SequenceIdGenerator {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SequenceIdGenerator.class);
+ private static final long INVALID_SEQUENCE_ID = 0;
+
+ static class Batch {
+ // The upper bound of the batch.
+ private long lastId = INVALID_SEQUENCE_ID;
+ // The next id to be allocated in this batch.
+ private long nextId = lastId + 1;
+ }
+
+ private final Map<String, Batch> sequenceIdToBatchMap;
+
+ private final Lock lock;
+ private final long batchSize;
+ private final StateManager stateManager;
+
+ /**
+ * @param conf : conf
+ * @param scmhaManager : null if non-Ratis based
+ * @param sequenceIdTable : sequenceIdTable
+ */
+ public SequenceIdGenerator(ConfigurationSource conf,
+ SCMHAManager scmhaManager, Table<String, Long> sequenceIdTable) {
+ this.sequenceIdToBatchMap = new HashMap<>();
+ this.lock = new ReentrantLock();
+ this.batchSize = conf.getInt(OZONE_SCM_SEQUENCE_ID_BATCH_SIZE,
+ OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT);
+
+ if (SCMHAUtils.isSCMHAEnabled(conf)) {
+ this.stateManager = new StateManagerHAImpl.Builder()
+ .setRatisServer(scmhaManager.getRatisServer())
+ .setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setSequenceIdTable(sequenceIdTable)
+ .build();
+ } else {
+ this.stateManager = new StateManagerImpl(sequenceIdTable);
+ }
+ }
+
+ /**
+ * @param sequenceIdName : name of the sequenceId
+ * @return : next id of this sequenceId.
+ */
+ public long getNextId(String sequenceIdName) {
+ lock.lock();
+ try {
+ Batch batch = sequenceIdToBatchMap.computeIfAbsent(
+ sequenceIdName, key -> new Batch());
+
+ if (batch.nextId <= batch.lastId) {
+ return batch.nextId++;
+ }
+
+ Preconditions.checkArgument(batch.nextId == batch.lastId + 1);
+ while (true) {
+ Long prevLastId = batch.lastId;
+ batch.nextId = prevLastId + 1;
+
+ Preconditions.checkArgument(Long.MAX_VALUE - batch.lastId >= batchSize);
+ batch.lastId += batchSize;
+
+ if (stateManager.allocateBatch(sequenceIdName,
+ prevLastId, batch.lastId)) {
+ LOG.info("Allocate a batch for {}, change lastId from {} to {}.",
+ sequenceIdName, prevLastId, batch.lastId);
+ break;
+ }
+
+ // reload lastId from RocksDB.
+ batch.lastId = stateManager.getLastId(sequenceIdName);
+ }
+
+ Preconditions.checkArgument(batch.nextId <= batch.lastId);
+ return batch.nextId++;
+
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Invalidate any un-exhausted batch, next getNextId() call will
+ * allocate a new batch.
+ */
+ public void invalidateBatch() {
+ lock.lock();
+ try {
+ sequenceIdToBatchMap.forEach(
+ (sequenceId, batch) -> batch.nextId = batch.lastId + 1);
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Maintain SequenceIdTable in RocksDB.
+ */
+ interface StateManager {
+ /**
+ * Compare And Swap lastId saved in db from expectedLastId to newLastId.
+ * If based on Ratis, it will submit a raft client request.
+ *
+ * @param sequenceIdName : name of the sequence id.
+ * @param expectedLastId : the expected lastId saved in db
+ * @param newLastId : the new lastId to save in db
+ * @return : result of the C.A.S.
+ */
+ @Replicate
+ Boolean allocateBatch(String sequenceIdName,
+ Long expectedLastId, Long newLastId);
+
+ /**
+ * @param sequenceIdName : name of the sequence id.
+ * @return lastId saved in db
+ */
+ Long getLastId(String sequenceIdName);
+ }
+
+ /**
+ * Ratis based StateManager, db operations are queued in
+ * DBTransactionBuffer until a snapshot is taken.
+ */
+ static final class StateManagerHAImpl implements StateManager {
+ private final Table<String, Long> sequenceIdTable;
+ private final DBTransactionBuffer transactionBuffer;
+ private final Map<String, Long> sequenceIdToLastIdMap;
+
+ private StateManagerHAImpl(Table<String, Long> sequenceIdTable,
+ DBTransactionBuffer trxBuffer) {
+ this.sequenceIdTable = sequenceIdTable;
+ this.transactionBuffer = trxBuffer;
+ this.sequenceIdToLastIdMap = new ConcurrentHashMap<>();
+ LOG.info("Init the HA SequenceIdGenerator.");
+ }
+
+ @Override
+ public Boolean allocateBatch(String sequenceIdName,
+ Long expectedLastId, Long newLastId) {
+ Long lastId = sequenceIdToLastIdMap.computeIfAbsent(sequenceIdName,
+ key -> {
+ try {
+ Long idInDb = this.sequenceIdTable.get(key);
+ return idInDb != null ? idInDb : INVALID_SEQUENCE_ID;
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to get lastId from db", ioe);
+ }
+ });
+
+ if (!lastId.equals(expectedLastId)) {
+ LOG.warn("Failed to allocate a batch for {}, expected lastId is {}," +
+ " actual lastId is {}.", sequenceIdName, expectedLastId, lastId);
+ return false;
+ }
+
+ try {
+ sequenceIdTable.putWithBatch(transactionBuffer
Review comment:
@bshashikant Please take care.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -0,0 +1,308 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.metadata.Replicate;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import static org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType.SEQUENCE_ID;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT;
+
+/**
+ * After SCM starts, set lastId = 0, nextId = lastId + 1.
+ * The first getNextId() call triggers SCM to load lastId from rocksDB,
+ * and allocate a new batch.
+ *
+ * In order to maintain monotonicity, for Ratis based SequenceIdGen,
+ * when becoming leader, SCM invalidates un-exhausted id batch by setting
+ * nextId = lastId + 1, so that a new leader will reload lastId from
+ * rocksDB and allocate a new batch when receiving its first getNextId() call.
+ */
+public class SequenceIdGenerator {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SequenceIdGenerator.class);
+ private static final long INVALID_SEQUENCE_ID = 0;
+
+ static class Batch {
+ // The upper bound of the batch.
+ private long lastId = INVALID_SEQUENCE_ID;
+ // The next id to be allocated in this batch.
+ private long nextId = lastId + 1;
+ }
+
+ private final Map<String, Batch> sequenceIdToBatchMap;
+
+ private final Lock lock;
+ private final long batchSize;
+ private final StateManager stateManager;
+
+ /**
+ * @param conf : conf
+ * @param scmhaManager : null if non-Ratis based
+ * @param sequenceIdTable : sequenceIdTable
+ */
+ public SequenceIdGenerator(ConfigurationSource conf,
+ SCMHAManager scmhaManager, Table<String, Long> sequenceIdTable) {
+ this.sequenceIdToBatchMap = new HashMap<>();
+ this.lock = new ReentrantLock();
+ this.batchSize = conf.getInt(OZONE_SCM_SEQUENCE_ID_BATCH_SIZE,
+ OZONE_SCM_SEQUENCE_ID_BATCH_SIZE_DEFAULT);
+
+ if (SCMHAUtils.isSCMHAEnabled(conf)) {
+ this.stateManager = new StateManagerHAImpl.Builder()
+ .setRatisServer(scmhaManager.getRatisServer())
+ .setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setSequenceIdTable(sequenceIdTable)
+ .build();
+ } else {
+ this.stateManager = new StateManagerImpl(sequenceIdTable);
+ }
+ }
+
+ /**
+ * @param sequenceIdName : name of the sequenceId
+ * @return : next id of this sequenceId.
+ */
+ public long getNextId(String sequenceIdName) {
+ lock.lock();
+ try {
+ Batch batch = sequenceIdToBatchMap.computeIfAbsent(
+ sequenceIdName, key -> new Batch());
+
+ if (batch.nextId <= batch.lastId) {
+ return batch.nextId++;
+ }
+
+ Preconditions.checkArgument(batch.nextId == batch.lastId + 1);
+ while (true) {
+ Long prevLastId = batch.lastId;
+ batch.nextId = prevLastId + 1;
+
+ Preconditions.checkArgument(Long.MAX_VALUE - batch.lastId >= batchSize);
+ batch.lastId += batchSize;
+
+ if (stateManager.allocateBatch(sequenceIdName,
+ prevLastId, batch.lastId)) {
+ LOG.info("Allocate a batch for {}, change lastId from {} to {}.",
+ sequenceIdName, prevLastId, batch.lastId);
+ break;
+ }
+
+ // reload lastId from RocksDB.
+ batch.lastId = stateManager.getLastId(sequenceIdName);
+ }
+
+ Preconditions.checkArgument(batch.nextId <= batch.lastId);
+ return batch.nextId++;
+
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Invalidate any un-exhausted batch, next getNextId() call will
+ * allocate a new batch.
+ */
+ public void invalidateBatch() {
+ lock.lock();
+ try {
+ sequenceIdToBatchMap.forEach(
+ (sequenceId, batch) -> batch.nextId = batch.lastId + 1);
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ /**
+ * Maintain SequenceIdTable in RocksDB.
+ */
+ interface StateManager {
+ /**
+ * Compare And Swap lastId saved in db from expectedLastId to newLastId.
+ * If based on Ratis, it will submit a raft client request.
+ *
+ * @param sequenceIdName : name of the sequence id.
+ * @param expectedLastId : the expected lastId saved in db
+ * @param newLastId : the new lastId to save in db
+ * @return : result of the C.A.S.
+ */
+ @Replicate
+ Boolean allocateBatch(String sequenceIdName,
+ Long expectedLastId, Long newLastId);
+
+ /**
+ * @param sequenceIdName : name of the sequence id.
+ * @return lastId saved in db
+ */
+ Long getLastId(String sequenceIdName);
+ }
+
+ /**
+ * Ratis based StateManager, db operations are queued in
+ * DBTransactionBuffer until a snapshot is taken.
+ */
+ static final class StateManagerHAImpl implements StateManager {
+ private final Table<String, Long> sequenceIdTable;
+ private final DBTransactionBuffer transactionBuffer;
+ private final Map<String, Long> sequenceIdToLastIdMap;
+
+ private StateManagerHAImpl(Table<String, Long> sequenceIdTable,
+ DBTransactionBuffer trxBuffer) {
+ this.sequenceIdTable = sequenceIdTable;
+ this.transactionBuffer = trxBuffer;
+ this.sequenceIdToLastIdMap = new ConcurrentHashMap<>();
+ LOG.info("Init the HA SequenceIdGenerator.");
+ }
+
+ @Override
+ public Boolean allocateBatch(String sequenceIdName,
+ Long expectedLastId, Long newLastId) {
+ Long lastId = sequenceIdToLastIdMap.computeIfAbsent(sequenceIdName,
+ key -> {
+ try {
+ Long idInDb = this.sequenceIdTable.get(key);
+ return idInDb != null ? idInDb : INVALID_SEQUENCE_ID;
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to get lastId from db", ioe);
+ }
+ });
+
+ if (!lastId.equals(expectedLastId)) {
+ LOG.warn("Failed to allocate a batch for {}, expected lastId is {}," +
+ " actual lastId is {}.", sequenceIdName, expectedLastId, lastId);
+ return false;
+ }
+
+ try {
+ sequenceIdTable.putWithBatch(transactionBuffer
+ .getCurrentBatchOperation(), sequenceIdName, newLastId);
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to put lastId to Batch", ioe);
+ }
+
+ sequenceIdToLastIdMap.put(sequenceIdName, newLastId);
+ return true;
+ }
+
+ @Override
+ public Long getLastId(String sequenceIdName) {
+ return sequenceIdToLastIdMap.get(sequenceIdName);
+ }
+
+ /**
+ * Builder for Ratis based StateManager.
+ */
+ public static class Builder {
+ private Table<String, Long> table;
+ private DBTransactionBuffer buffer;
+ private SCMRatisServer ratisServer;
+
+ public Builder setRatisServer(final SCMRatisServer scmRatisServer) {
+ this.ratisServer = scmRatisServer;
+ return this;
+ }
+
+ public Builder setSequenceIdTable(
+ final Table<String, Long> sequenceIdTable) {
+ table = sequenceIdTable;
+ return this;
+ }
+
+ public Builder setDBTransactionBuffer(DBTransactionBuffer trxBuffer) {
+ buffer = trxBuffer;
+ return this;
+ }
+
+ public StateManager build() {
+ Preconditions.checkNotNull(table);
+ Preconditions.checkNotNull(buffer);
+ Preconditions.checkNotNull(ratisServer);
Review comment:
@bshashikant Please take care.
----------------------------------------------------------------
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 edited a comment on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng edited a comment on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789689523
> Do we need to handle overflow for "long" used here for lastId?
Thanks for the advice, I've add the check.
> With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
> We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
We have a discussion in the design doc:
> need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
localId : the initial value should be UniqueId() + 1
containerID: scan the container db, figure the largest containerID ever used.
delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
So, we can simply use `UniqueId() + 1` as `INVALID_SEQUENCE_ID`, since containerID and delTx are starting from 1, practically they can not be larger than `UniqueId() + 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] bshashikant commented on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789701900
> > Do we need to handle overflow for "long" used here for lastId?
>
> Thanks for the advice, I've add the check.
>
> > With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
>
> Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
What i meant was, containerIds are not sequentially increasing which is a departure from the current behaviour which is strictly sequentially increasing containerIds. and we are using the same seqID generator for both Container as well as block.
This may become the reason for scale limitation.
Instead of that, why can't we use lastAppliedIndex as base for containerIds.
>
> > We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
>
> We have a discussion in the design doc:
>
> > need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
> > localId : the initial value should be UniqueId() + 1
> > containerID: scan the container db, figure the largest containerID ever used.
> > delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
>
> So, we can simply use `UniqueId() + 1` as `INVALID_SEQUENCE_ID`, since containerID and delTx are starting from 1, practically they can not be larger than `UniqueId() + 1`.
Are u going to address these changes in subsequent PR as this will be required for branch merge?
----------------------------------------------------------------
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 edited a comment on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng edited a comment on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789689523
> Do we need to handle overflow for "long" used here for lastId?
Thanks for the advice, I've add the check.
> With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
> We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
We have a discussion in the design doc:
> need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
localId : the initial value should be UniqueId() + 1
containerID: scan the container db, figure the largest containerID ever used.
delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
So, we can simply use `UniqueId() + 1`, since containerID and delTx are starting from 1, they can not be larger than `UniqueId() + 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 edited a comment on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng edited a comment on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789689523
> Do we need to handle overflow for "long" used here for lastId?
Thanks for the advice, I've add the check.
> With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
> We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
We have a discussion in the design doc:
> need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
localId : the initial value should be UniqueId() + 1
containerID: scan the container db, figure the largest containerID ever used.
delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
So, we can simply use `UniqueId() + 1` as `INVALID_SEQUENCE_ID`, since containerID and delTx are starting from 1, they can not be larger than `UniqueId() + 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] amaliujia commented on a change in pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#discussion_r586114369
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSequenceIDGen.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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
+ * <p>
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * <p>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.ha;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore;
+import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStoreImpl;
+import org.apache.hadoop.ozone.container.common.SCMTestUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SEQUENCE_ID_BATCH_SIZE;
+
+public class TestSequenceIDGen {
+ @Test
+ public void testSequenceIDGenUponNonRatis() throws Exception {
Review comment:
Do we have SCM HA in MiniOzoneCluster now? If so it will be great to have a leader switching tests?
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#discussion_r586114018
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/BooleanCodec.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.ha.io;
+
+import com.google.protobuf.ByteString;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class BooleanCodec implements Codec {
Review comment:
Why do you need BoloeanCodec? Is it used?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/BooleanCodec.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.ha.io;
+
+import com.google.protobuf.ByteString;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class BooleanCodec implements Codec {
Review comment:
Why do you need BooleanCodec? Is it used?
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
bshashikant merged pull request #1980:
URL: https://github.com/apache/ozone/pull/1980
----------------------------------------------------------------
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 #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789689523
> Do we need to handle overflow for "long" used here for lastId?
Thanks for the advice, I've add the check.
> With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
> We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
We have a discussion in the design doc:
> need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
localId : the initial value should be UniqueId() + 1
containerID: scan the container db, figure the largest containerID ever used.
delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
So, we can simply use `UniqueId() + 1`, since containerID and delTx can not be larger than `UniqueId() + 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] bshashikant commented on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-791203566
The upgrade work will be handled in a different PR. Thanks @GlenGeng for the work.
----------------------------------------------------------------
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