Skip to content

HDDS-8458. Intermittent timeout in TestBlockDeletion#testBlockDeletion#4624

Merged
adoroszlai merged 3 commits intoapache:masterfrom
sumitagrawl:HDDS-8458
Apr 28, 2023
Merged

HDDS-8458. Intermittent timeout in TestBlockDeletion#testBlockDeletion#4624
adoroszlai merged 3 commits intoapache:masterfrom
sumitagrawl:HDDS-8458

Conversation

@sumitagrawl
Copy link
Contributor

What changes were proposed in this pull request?

Thread sleep replaced with waiting for DN container to Closed

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8458

How was this patch tested?

Flaky test fix, test case running multiple time

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sumitagrawl for the patch.

In addition to the inline comment, you can also remove @Flaky("HDDS-8458") from line 190.

Comment on lines +251 to +260
GenericTestUtils.waitFor(() -> {
return !(omKeyLocationInfoGroupList.stream().filter((group) ->
group.getLocationList().stream().filter(
(info) -> cluster.getHddsDatanodes().get(0)
.getDatanodeStateMachine().getContainer().getContainerSet()
.getContainer(info.getContainerID()).getContainerData()
.getState() != ContainerProtos.ContainerDataProto.State.CLOSED)
.findFirst().isPresent()
).findFirst().isPresent());
}, 1000, 30000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified:

  1. .filter(...).findFirst().isPresent() -> anyMatch(...)
  2. !anyMatch() -> noneMatch()
  3. noneMatch(... != ...) -> allMatch(... == ...)
  4. inner stream -> flatMap
  5. { return ... } -> ...
  6. store ContainerSet in local variable

Like this:

    ContainerSet containerSet = cluster.getHddsDatanodes().get(0)
        .getDatanodeStateMachine().getContainer().getContainerSet();
    GenericTestUtils.waitFor(() -> omKeyLocationInfoGroupList.stream()
        .flatMap(group -> group.getLocationList().stream())
        .allMatch(info -> containerSet.getContainer(info.getContainerID())
            .getContainerData()
            .getState() == ContainerProtos.ContainerDataProto.State.CLOSED),
        1000, 30000);

Please double-check for any mistakes I may have made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated...

@sumitagrawl sumitagrawl requested a review from adoroszlai April 28, 2023 04:25
@adoroszlai adoroszlai merged commit fa83195 into apache:master Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants