Uh oh!
There was an error while loading. Please reload this page.
HDDS-8848. Clean up datanode memory in case of an ozone stream write error - #4891
Conversation
guohao-rosicky
commented
Jun 14, 2023
hi, @szetszwo . |
guohao-rosicky
commented
Jun 14, 2023
cc @ChenSammi |
szetszwo
left a comment
There was a problem hiding this comment.
@guohao-rosicky , thanks a lot for working on this! Please see the comments inlined.
| } | ||
| final ContainerCommandRequestProto request; | ||
| if (dataChannel instanceof KeyValueStreamDataChannel) { |
There was a problem hiding this comment.
Let's change it to
if (!(dataChannelinstanceofKeyValueStreamDataChannel)) {
returnJavaUtils.completeExceptionally(newIllegalStateException(
"Unexpected DataChannel " + dataChannel.getClass()));
}Then, the code don't need to have so many indentation levels.
| class LocalStream implements StateMachine.DataStream { | ||
| private final StateMachine.DataChannel dataChannel; | ||
| private final StreamDataChannelBase dataChannel; |
There was a problem hiding this comment.
Use KeyValueStreamDataChannel instead. Then, we can keep StreamDataChannelBase package private.
| void cleanUpAll() { | ||
| final int size = deque.size(); | ||
| for (int i = 0; i < size; i++) { | ||
| Optional.ofNullable(poll()).ifPresent(ReferenceCountedObject::release); | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's call it releaseAll() and check isEmpty(). Also, poll() won't return null.
voidreleaseAll() {
while (!deque.isEmpty()) {
poll().release();
}
}| @Override | ||
| protected void cleanupInternal() throws IOException { | ||
| buffers.cleanUpAll(); | ||
| if (!closed.get()) { |
There was a problem hiding this comment.
We should use if (closed.compareAndSet(false, true)) {
| * For write state machine data. | ||
| */ | ||
| abstract class StreamDataChannelBase implements StateMachine.DataChannel { | ||
| public abstract class StreamDataChannelBase |
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
What changes were proposed in this pull request?
Clean up datanode memory in case of an ozone stream write error
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8848
How was this patch tested?