Skip to content

MINOR: Remove redudant LocalLogManager#10325

Merged
cmccabe merged 1 commit intoapache:trunkfrom
dengziming:minor-remove-redudant-LocalLogManager
Apr 2, 2021
Merged

MINOR: Remove redudant LocalLogManager#10325
cmccabe merged 1 commit intoapache:trunkfrom
dengziming:minor-remove-redudant-LocalLogManager

Conversation

@dengziming
Copy link
Member

More detailed description of your change
We have 2 identical LocalLogManager in src/main/org.apache.kafka.metalog and src/test/org.apache.kafka.metalog, the only difference is that the test class have more docs, I just move the docs from test to src and remove test class.

Summary of testing strategy (including rationale)
QA

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dengziming
Copy link
Member Author

ping @cmccabe @mumrah to have a look.

@dengziming dengziming changed the title MONOR: Remove redudant LocalLogManager MINOR: Remove redudant LocalLogManager Mar 16, 2021
@ijuma ijuma requested a review from cmccabe March 18, 2021 14:18
@dengziming dengziming force-pushed the minor-remove-redudant-LocalLogManager branch from a57fb25 to 4da87ea Compare March 22, 2021 06:27
@cmccabe
Copy link
Contributor

cmccabe commented Mar 22, 2021

Thanks for spotting this, @dengziming . I thought we fixed this already, but I guess not.

The correct fix is to delete this code completely from the main directory, since it is used only for testing. This PR seems to do the reverse. Can you please change it around so the code under metadata/src/main is deleted?

Also, we want to keep the comments (or to put another way, we don't need to change the version in metadata/src/test at all)

@cmccabe cmccabe added the kraft label Mar 22, 2021
@dengziming dengziming force-pushed the minor-remove-redudant-LocalLogManager branch from 4da87ea to e564a3a Compare March 23, 2021 00:58
@dengziming
Copy link
Member Author

@cmccabe done! Thank you for your reply, PTAL again.

Copy link
Contributor

@cmccabe cmccabe left a comment

Choose a reason for hiding this comment

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

LGTM

@cmccabe cmccabe merged commit 798672f into apache:trunk Apr 2, 2021
ijuma added a commit to ijuma/kafka that referenced this pull request Apr 4, 2021
…e-allocations-lz4

* apache-github/trunk: (243 commits)
  KAFKA-12590: Remove deprecated kafka.security.auth.Authorizer, SimpleAclAuthorizer and related classes in 3.0 (apache#10450)
  KAFKA-3968: fsync the parent directory of a segment file when the file is created (apache#10405)
  KAFKA-12283: disable flaky testMultipleWorkersRejoining to stabilize build (apache#10408)
  MINOR: remove KTable.to from the docs (apache#10464)
  MONOR: Remove redudant LocalLogManager (apache#10325)
  MINOR: support ImplicitLinkedHashCollection#sort (apache#10456)
  KAFKA-12587 Remove KafkaPrincipal#fromString for 3.0 (apache#10447)
  KAFKA-12426: Missing logic to create partition.metadata files in RaftReplicaManager (apache#10282)
  MINOR: Improve reproducability of raft simulation tests (apache#10422)
  KAFKA-12474: Handle failure to write new session keys gracefully (apache#10396)
  KAFKA-12593: Fix Apache License headers (apache#10452)
  MINOR: Fix typo in MirrorMaker v2 documentation (apache#10433)
  KAFKA-12600: Remove deprecated config value `default` for client config `client.dns.lookup` (apache#10458)
  KAFKA-12952: Remove deprecated LogConfig.Compact (apache#10451)
  Initial commit (apache#10454)
  KAFKA-12575: Eliminate Log.isLogDirOffline boolean attribute (apache#10430)
  KAFKA-8405; Remove deprecated `kafka-preferred-replica-election` command (apache#10443)
  MINOR: Fix docs for end-to-end record latency metrics (apache#10449)
  MINOR Replaced File with Path in LogSegmentData. (apache#10424)
  KAFKA-12583: Upgrade netty to 4.1.62.Final
  ...
@dengziming dengziming deleted the minor-remove-redudant-LocalLogManager branch August 28, 2021 14:11
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