Skip to content

KAFKA-13068: Rename Log to UnifiedLog#11154

Merged
junrao merged 5 commits intoapache:trunkfrom
kowshik:KAFKA-13068_rename_Log_to_UnifiedLog
Aug 12, 2021
Merged

KAFKA-13068: Rename Log to UnifiedLog#11154
junrao merged 5 commits intoapache:trunkfrom
kowshik:KAFKA-13068_rename_Log_to_UnifiedLog

Conversation

@kowshik
Copy link
Copy Markdown
Contributor

@kowshik kowshik commented Jul 31, 2021

In this PR, I've renamed kafka.log.Log to kafka.log.UnifiedLog. With the advent of KIP-405, going forward the existing Log class would present a unified view of local and tiered log segments, so we rename it to UnifiedLog. The motivation for this PR is also the same as outlined in this design document: https://docs.google.com/document/d/1dQJL4MCwqQJSPmZkVmVzshFZKuFy_bCPtubav4wBfHQ/edit.
This PR is a follow-up to #10280 where we had refactored the Log layer introducing a new kafka.log.LocalLog class.

Note: the Log class name had to be hardcoded to ensure metrics are defined under the Log class (for backwards compatibility). Please refer to the newly introduced UnifiedLog.metricName() method.

Tests:
Relying on existing unit & integration tests to surface any regressions.

@kowshik
Copy link
Copy Markdown
Contributor Author

kowshik commented Jul 31, 2021

cc @junrao @ccding @satishd - this PR is ready for review.

Copy link
Copy Markdown
Contributor

@ccding ccding left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM

Comment thread core/src/main/scala/kafka/log/UnifiedLog.scala Outdated
Copy link
Copy Markdown
Member

@satishd satishd left a comment

Choose a reason for hiding this comment

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

Thanks @kowshik for the PR, LGTM.

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@kowshik : Thanks for the PR. Just a minor comment below.

Also. Scala 2.12 tests timed out. Is that related to this PR?

Comment thread core/src/main/scala/kafka/log/UnifiedLog.scala Outdated
@kowshik kowshik force-pushed the KAFKA-13068_rename_Log_to_UnifiedLog branch from 5a85097 to 57a8bc3 Compare August 5, 2021 06:46
Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@kowshik : Thanks for the updated PR. One more comment below.

Comment thread core/src/main/scala/kafka/log/UnifiedLog.scala Outdated
@kowshik kowshik force-pushed the KAFKA-13068_rename_Log_to_UnifiedLog branch from 61ae17c to 360d62b Compare August 7, 2021 07:38
Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@kowshik : Thanks for the updated PR. It seems there is compilation error?

@kowshik
Copy link
Copy Markdown
Contributor Author

kowshik commented Aug 9, 2021

@junrao Thanks for the review! I've fixed the build failure and uploaded 6d30c102a4816803f51c5b2c2350aa75b51238f2. We can wait for CI to pass.

@kowshik kowshik force-pushed the KAFKA-13068_rename_Log_to_UnifiedLog branch from 6d30c10 to 46092a8 Compare August 9, 2021 22:03
@kowshik
Copy link
Copy Markdown
Contributor Author

kowshik commented Aug 10, 2021

@junrao I checked CI results. The build has passed and the failing tests don't look related to this PR.

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@kowshik : Thanks for the PR. LGTM

@junrao junrao merged commit db1f581 into apache:trunk Aug 12, 2021
xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
In this PR, I've renamed kafka.log.Log to kafka.log.UnifiedLog. With the advent of KIP-405, going forward the existing Log class would present a unified view of local and tiered log segments, so we rename it to UnifiedLog. The motivation for this PR is also the same as outlined in this design document: https://docs.google.com/document/d/1dQJL4MCwqQJSPmZkVmVzshFZKuFy_bCPtubav4wBfHQ/edit.
This PR is a follow-up to apache#10280 where we had refactored the Log layer introducing a new kafka.log.LocalLog class.

Note: the Log class name had to be hardcoded to ensure metrics are defined under the Log class (for backwards compatibility). Please refer to the newly introduced UnifiedLog.metricName() method.

Reviewers: Cong Ding <cong@ccding.com>, Satish Duggana <satishd@apache.org>, Jun Rao <junrao@gmail.com>
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.

4 participants