Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #6525]Make ConsumeQueueInterface extends from FileQueueLifeCycle #6534

Merged
merged 7 commits into from
Apr 15, 2023

Conversation

Abhijeetmishr
Copy link
Contributor

@Abhijeetmishr Abhijeetmishr commented Apr 3, 2023

Make sure set the target branch to develop

What is the purpose of the change

close #6525
we are typecasting here

 private FileQueueLifeCycle getLifeCycle(String topic, int queueId) {
        return (FileQueueLifeCycle) findOrCreateConsumeQueue(topic, queueId);
    }

but interface is not extending FileQueueLifeCycle.
So it will throw typecast error.

@ni-ze ni-ze changed the title Make ConsumeQueueInterface extends from FileQueueLifeCycle [ISSUE #6525]Make ConsumeQueueInterface extends from FileQueueLifeCycle Apr 4, 2023
@ni-ze
Copy link
Contributor

ni-ze commented Apr 4, 2023

  1. ConsumeQueue also need to modify;
  2. The forced convert need to remove.

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration .github/workflows/codeql_analysis.yml:CodeQL-Build. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2023

Codecov Report

Merging #6534 (77d8357) into develop (d9a7315) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@              Coverage Diff              @@
##             develop    #6534      +/-   ##
=============================================
- Coverage      43.07%   43.06%   -0.01%     
+ Complexity      8992     8988       -4     
=============================================
  Files           1107     1107              
  Lines          78260    78260              
  Branches       10201    10201              
=============================================
- Hits           33713    33706       -7     
- Misses         40318    40325       +7     
  Partials        4229     4229              
Impacted Files Coverage Δ
...apache/rocketmq/store/queue/BatchConsumeQueue.java 70.21% <ø> (ø)
...apache/rocketmq/store/queue/ConsumeQueueStore.java 48.20% <50.00%> (ø)

... and 25 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Abhijeetmishr
Copy link
Contributor Author

  1. ConsumeQueue also need to modify;
  2. The forced convert need to remove.

@ni-ze okay got it.

@lizhimins
Copy link
Member

赞,我之前也想改这里来着!

@Abhijeetmishr
Copy link
Contributor Author

Praise, I also wanted to change here before!

@lizhimins didn't get you ?

@Abhijeetmishr
Copy link
Contributor Author

Abhijeetmishr commented Apr 6, 2023

hey @Oliverwqcwrw @ni-ze I can see some build failures, how do I resolve them any hint/help is appreciated ?

@ni-ze
Copy link
Contributor

ni-ze commented Apr 12, 2023

@Abhijeetmishr
The build is almost pass, plz check the review suggestion and resolve it.

@Abhijeetmishr
Copy link
Contributor Author

  • ConsumeQueue also need to modify;
  • The forced convert need to remove.
  1. ConsumeQueue also need to modify; - checking on this
  2. The forced convert need to remove. - this is done already

@ni-ze
Copy link
Contributor

ni-ze commented Apr 13, 2023

  • ConsumeQueue also need to modify;
  • The forced convert need to remove.
  1. ConsumeQueue also need to modify; - checking on this
  2. The forced convert need to remove. - this is done already

There is another force convert in ConsumeQueueStore.

@ni-ze
Copy link
Contributor

ni-ze commented Apr 13, 2023

@Abhijeetmishr Merge develop into your branch, and then submit pr again, checks will be success.

@Abhijeetmishr
Copy link
Contributor Author

Abhijeetmishr commented Apr 13, 2023

@ni-ze Merge develop into your branch :- you mean to say that take pull from develop branch to my branch and push along with changes?

@@ -79,7 +79,7 @@ public void setTopicConfigTable(ConcurrentMap<String, TopicConfig> topicConfigTa
}

private FileQueueLifeCycle getLifeCycle(String topic, int queueId) {
return (FileQueueLifeCycle) findOrCreateConsumeQueue(topic, queueId);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is another force convert in recoverConcurrently method of this class.

@ni-ze ni-ze merged commit d1b14b0 into apache:develop Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make ConsumeQueueInterface extends from FileQueueLifeCycle
4 participants