szetszwo commented on code in PR #10823:
URL: https://github.com/apache/ozone/pull/10823#discussion_r3667949480
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -209,6 +209,20 @@ public long persistPutBlock(KeyValueContainer container,
// container to determine whether the blockCount is already incremented
// for this block in the DB or not.
long localID = data.getLocalID();
+ // For the PutBlock that is endOfBlock and meanwhile bscId = 0, it means
+ // this PutBlock comes from data stream close without going through the
+ // Ratis, thus there is no log index. In this case, we should not let
+ // 0 to overwrite previous possible PutBlocks from Ratis log that were
+ // generated during immediate flushes from an active data stream.
Instead,
+ // we should load the latest bscid and reuse that id.
+ if (endOfBlock && bcsId == 0) {
+ BlockData existing = db.getStore().getBlockDataTable()
+ .get(containerData.getBlockKey(localID));
+ if (existing != null) {
+ bcsId = existing.getBlockCommitSequenceId();
Review Comment:
Must existing.getBlockCommitSequenceId() be 0 ? It seems impossible to have
another value. If it is the case, let's add a Preconditions.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java:
##########
@@ -91,6 +91,14 @@ public boolean addAll(@Nullable Collection<OzoneAcl>
newAcls) {
return added;
}
+ /**
+ * Overload accepting List instead of Collection for binary compatibility
+ * with callers compiled against {@code set(List)}.
+ */
+ public boolean set(@Nonnull List<OzoneAcl> acls) {
Review Comment:
Which test will fail? Since the PR builds and other people don't see this
problem, would it be the case that there some old artifacts? Could you try
rebuilding everything?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -209,6 +209,20 @@ public long persistPutBlock(KeyValueContainer container,
// container to determine whether the blockCount is already incremented
// for this block in the DB or not.
long localID = data.getLocalID();
+ // For the PutBlock that is endOfBlock and meanwhile bscId = 0, it means
+ // this PutBlock comes from data stream close without going through the
+ // Ratis, thus there is no log index. In this case, we should not let
Review Comment:
Let's use Raft instead of Ratis since Streaming is a also a part of Ratis.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
