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/09/03 06:52:39 UTC
[GitHub] [ozone] JacksonYao287 opened a new pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
JacksonYao287 opened a new pull request #2611:
URL: https://github.com/apache/ozone/pull/2611
## What changes were proposed in this pull request?
2021-09-03 12:50:38,632 [1114efaf-c975-4ea0-bc8b-c3309a57cc8d@group-58D600BFB107-StateMachineUpdater] WARN org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator: Failed to allocate a batch for localId, expected lastId is 0, actual lastId is 107544261438510000.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5710
## How was this patch tested?
current unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 edited a comment on pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
JacksonYao287 edited a comment on pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#issuecomment-914268170
> Can you explain the problem in bit detail here?
thanks @bshashikant for the review! when i investigate some issues in our production cluster, i find the log below, which is in
`SequenceIdGenerator.java`
2021-09-03 12:50:38,632 [1114efaf-c975-4ea0-bc8b-c3309a57cc8d@group-58D600BFB107-StateMachineUpdater] WARN org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator: Failed to allocate a batch for localId, **expected lastId is 0**, actual lastId is 107544261438510000.
let us see the code below
```
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;
}
```
`expected lastId is 0` means `sequenceIdToLastIdMap` does not contains `sequenceIdName`(localId), and thus `INVALID_SEQUENCE_ID`(0) is assigned to `lastId`. this is a long run ozone cluster , so local id should exist in sequenceIdToLastIdMap.
for now , when leader reelection happens , the new leader will call `reinitialize` , but in current `SequenceIdGenerator` does not initialize `sequenceIdToLastIdMap` , which hold the latest state of `lastId`. so this is what this patch does, similar as `containerStateManagerImpl#initialize`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#issuecomment-914268170
> Can you explain the problem in bit detail here?
thanks @bshashikant for the review! when i investigate some issues in our production cluster, i find the log below, which is in
`SequenceIdGenerator.java`
2021-09-03 12:50:38,632 [1114efaf-c975-4ea0-bc8b-c3309a57cc8d@group-58D600BFB107-StateMachineUpdater] WARN org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator: Failed to allocate a batch for localId, **expected lastId is 0**, actual lastId is 107544261438510000.
let us see the code below
```
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;
}
```
`expected lastId is 0` means `sequenceIdToLastIdMap` does not contains `sequenceIdName`(localId), and thus `INVALID_SEQUENCE_ID`(0) is assigned to `lastId`. this is a long run ozone cluster , so local id should exist in sequenceIdToLastIdMap.
for now , when leader reelection happens , the new leader will call `reinitialize` , but in current `SequenceIdGenerator` does not initialize `sequenceIdToLastIdMap` , which hold the latest state of `lastId`. so this is what this patch does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#issuecomment-932281046
@bshashikant thanks for the review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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 #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
bshashikant merged pull request #2611:
URL: https://github.com/apache/ozone/pull/2611
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on a change in pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#discussion_r703451357
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -253,9 +254,28 @@ public Long getLastId(String sequenceIdName) {
}
@Override
- public void reinitialize(Table<String, Long> seqIdTable) {
+ public void reinitialize(Table<String, Long> seqIdTable)
+ throws IOException {
this.sequenceIdTable = seqIdTable;
this.sequenceIdToLastIdMap.clear();
+ initialize();
+ }
+
+ private void initialize() throws IOException {
+ TableIterator<String,
+ ? extends Table.KeyValue<String, Long>>
+ iterator = sequenceIdTable.iterator();
+
+ while (iterator.hasNext()) {
+ Table.KeyValue<String, Long> kv = iterator.next();
+ final String sequenceIdName = kv.getKey();
+ final Long lastId = kv.getValue();
+ org.apache.ratis.util.Preconditions.assertNotNull(sequenceIdName,
Review comment:
sure, will do 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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 #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#issuecomment-914229867
Can you explain the problem in bit detail here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on pull request #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#issuecomment-912496117
seems a flaky case , not caused by this patch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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 #2611: HDDS-5710. initialize sequenceIdToLastIdMap when SequenceIdGenerator#StateManager reinitializes
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2611:
URL: https://github.com/apache/ozone/pull/2611#discussion_r703431861
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java
##########
@@ -253,9 +254,28 @@ public Long getLastId(String sequenceIdName) {
}
@Override
- public void reinitialize(Table<String, Long> seqIdTable) {
+ public void reinitialize(Table<String, Long> seqIdTable)
+ throws IOException {
this.sequenceIdTable = seqIdTable;
this.sequenceIdToLastIdMap.clear();
+ initialize();
+ }
+
+ private void initialize() throws IOException {
+ TableIterator<String,
+ ? extends Table.KeyValue<String, Long>>
+ iterator = sequenceIdTable.iterator();
+
+ while (iterator.hasNext()) {
+ Table.KeyValue<String, Long> kv = iterator.next();
+ final String sequenceIdName = kv.getKey();
+ final Long lastId = kv.getValue();
+ org.apache.ratis.util.Preconditions.assertNotNull(sequenceIdName,
Review comment:
Can u fix the import and use just Preconditions check?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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