Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

[branch-2.11] Fix failed GroupMetadataManagerTest #1934

Merged

Conversation

BewareMyPower
Copy link
Collaborator

Motivation

When running GroupMetadataManagerTest,
testOffsetWriteAfterGroupRemoved will be called after testOffsetTopicNumPartitionsModify, which changes the offsetTopicNumPartitions to 100 so that consumerGroupPartitionId will be 74 while groupPartitionId is 0, and then the group metadata will be sent to partition 0 while scheduleLoadGroupAndOffsets will read from partition 74. Since no group metadata is loaded,

Modifications

Migrate the fix in #1759 to reset the offsetTopicNumPartitions before and after each method.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

### Motivation

When running `GroupMetadataManagerTest`,
`testOffsetWriteAfterGroupRemoved` will be called after
`testOffsetTopicNumPartitionsModify`, which changes the
`offsetTopicNumPartitions` to 100 so that `consumerGroupPartitionId`
will be 74 while `groupPartitionId` is 0, and then the group metadata
will be sent to partition 0 while `scheduleLoadGroupAndOffsets` will
read from partition 74. Since no group metadata is loaded,

### Modifications

Migrate the fix in #1759 to
reset the `offsetTopicNumPartitions` before and after each method.
@BewareMyPower BewareMyPower requested review from jiazhai and a team as code owners July 1, 2023 14:15
@BewareMyPower BewareMyPower self-assigned this Jul 1, 2023
@github-actions github-actions bot added the no-need-doc This pr does not need any document label Jul 1, 2023
@codecov
Copy link

codecov bot commented Jul 1, 2023

Codecov Report

Merging #1934 (ada5f47) into branch-2.11 (6da90f2) will decrease coverage by 0.27%.
The diff coverage is 3.67%.

Impacted file tree graph

@@                Coverage Diff                @@
##             branch-2.11    #1934      +/-   ##
=================================================
- Coverage          18.37%   18.11%   -0.27%     
  Complexity           707      707              
=================================================
  Files                173      178       +5     
  Lines              13065    13301     +236     
  Branches            1195     1232      +37     
=================================================
+ Hits                2401     2409       +8     
- Misses             10495    10721     +226     
- Partials             169      171       +2     
Impacted Files Coverage Δ
...ative/pulsar/handlers/kop/KafkaCommandDecoder.java 0.32% <0.00%> (-0.01%) ⬇️
...tive/pulsar/handlers/kop/KafkaProtocolHandler.java 0.00% <0.00%> (ø)
...ative/pulsar/handlers/kop/KafkaRequestHandler.java 1.08% <0.00%> (-0.01%) ⬇️
...pulsar/handlers/kop/KafkaTopicConsumerManager.java 0.00% <ø> (ø)
...eamnative/pulsar/handlers/kop/KopEventManager.java 7.21% <0.00%> (ø)
...mnative/pulsar/handlers/kop/SystemTopicClient.java 0.00% <0.00%> (ø)
...ers/kop/coordinator/CompactedPartitionedTopic.java 0.00% <0.00%> (ø)
...ndlers/kop/coordinator/group/GroupCoordinator.java 0.00% <ø> (ø)
.../kop/coordinator/group/GroupMetadataConstants.java 0.00% <0.00%> (ø)
...rs/kop/coordinator/group/GroupMetadataManager.java 0.00% <0.00%> (ø)
... and 19 more

@Demogorgon314 Demogorgon314 merged commit ae5d500 into branch-2.11 Jul 2, 2023
@Demogorgon314 Demogorgon314 deleted the bewaremypower/branch-2.11-fix-group-metadata-test branch July 2, 2023 00:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no-need-doc This pr does not need any document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants