Hi PoAn:

Thanks for your KIP. It is very useful. Though you already comment for the
naming issue. I just want to comment more for this:

I also think localOnlyLogSize is better than onlyLocalLogSize?
localOnlyLogSize is more easier for user to understand it when it is used
for API response.
The naming onlyLocalLogSize is ambiguous because the word "only" could be
modifying either "local" or "log", making the intended meaning unclear.

for "UnifiedLog#onlyLocalLogSegmentSize'. maybe it isn't a good example to
follow.

anyway,  Just my small opinion.

Thanks !

Regards
Jian

PoAn Yang <[email protected]> 于2025年10月31日周五 16:47写道:

> Hi Chia-Ping and Kamal,
>
> Sorry for the late reply. Thanks for the review.
>
> chia_06: I add an example in DescribeLogDirsResponse section. It explains
> difference
> among PartitionSize, RemoteLogSize, and OnlyLocalLogSize.
>
> Kind regards,
> PoAn
>
> > On Oct 25, 2025, at 12:40 PM, Kamal Chandraprakash <
> [email protected]> wrote:
> >
> > Ping to revive this KIP. Thanks!
> >
> > On Thu, Aug 14, 2025 at 6:50 AM Chia-Ping Tsai <[email protected]>
> wrote:
> >
> >> hi PoAn
> >>
> >> chia_06:
> >>
> >> As we discussed offline, `size`and `onlyLocalSize` could be difficult to
> >> understand when remote storage is enabled. It would be helpful if you
> could
> >> include some examples in the docs.
> >>
> >> Best,
> >> Chia-Ping
> >>
> >> PoAn Yang <[email protected]> 於 2025年8月5日 週二 下午11:08寫道:
> >>
> >>> Hi Kamal, Chia-Ping, and Satish,
> >>>
> >>> Thanks for the review and suggestions.
> >>>
> >>> Moved `IncludeRemoteInfo` out of `DescribableLogDirTopics`.
> >>>
> >>> chia_03: `PartitionSize != RemoteLogSize + OnlyLocalLogSize`.
> >>> The PartitionSize is all local log segments size. It includes part
> >>> of remote log segments which haven’t meet local retention.
> >>>
> >>> chia_04: I prefer to use `onlyLocalSize`, because it gets value
> >>> from UnifiedLog#onlyLocalLogSegmentSize. If we use `localSize`,
> >>> users may be think that `size = remoteSize + localSize`.
> >>>
> >>> chia_05: Change both `remoteSize` and `onlyLocalSize` to
> >>> optional long.
> >>>
> >>> 100: Updated the description of `RemoteLogSize`.
> >>>
> >>> 101: I prefer to align the naming order like
> >>> UnifiedLog#onlyLocalLogSegmentSize.
> >>> Updated the description of `onlyLocalSize`.
> >>>
> >>> Thanks,
> >>> PoAn
> >>>
> >>>> On Aug 5, 2025, at 1:29 PM, Satish Duggana <[email protected]>
> >>> wrote:
> >>>>
> >>>> Hi PoAn,
> >>>> Thanks for the KIP, this is a valuable feature for operators to get
> >>>> better visibility into partition sizes across both local and remote
> >>>> storage. While RemoteLogSegmentMetadata provides segment-level details
> >>>> to help developers build custom utilities, it's beneficial to enhance
> >>>> the existing Kafka utilities to surface more operationally useful
> >>>> information.
> >>>>
> >>>> Overall proposal looks good to me. I have a couple of minor comments.
> >>>>
> >>>> 100. RemoteLogSize – It would be helpful to enhance the description
> >>>> with more detail. For example:
> >>>> "The size of the remote log segments for this partition, in bytes.
> >>>> Note that some of these segments may still be present in the broker’s
> >>>> local storage."
> >>>>
> >>>> 101. onlyLocalSize – I suggest renaming this to LocalOnlySize for
> >>>> better clarity. The description can also be made more explicit, such
> >>>> as:
> >>>> "The size of the log segments stored only in the broker’s local
> >>>> storage for this partition, in bytes. This excludes any data that has
> >>>> been offloaded to remote storage."
> >>>>
> >>>> ~Satish.
> >>>>
> >>>> On Mon, 4 Aug 2025 at 15:57, Chia-Ping Tsai <[email protected]>
> >> wrote:
> >>>>>
> >>>>> hi PoAn
> >>>>>
> >>>>> chia_03: what is the difference between `OnlyLocalLogSize` and
> >>> `PartitionSize`? Am I correct in assuming that `PartitionSize` =
> >>> RemoteLogSize + OnlyLocalLogSize?
> >>>>>
> >>>>> chia_04: could you please consider renaming `onlyLocalSize` to
> >>> `localSize` for consistency?
> >>>>>
> >>>>> chia_05: should we use optional int as returned type for `remoteSize`
> >>> and `onlyLocalSize`? If not, could you please add comments to explain
> the
> >>> use of "-1"?
> >>>>>
> >>>>> Best,
> >>>>> Chia-Ping
> >>>>>
> >>>>>
> >>>>> On 2025/06/16 14:32:34 PoAn Yang wrote:
> >>>>>> Hi all,
> >>>>>>
> >>>>>> I would like to start a discussion thread about KIP-1187.
> >>>>>>
> >>>>>> Please take a look and feel free to share any thought.
> >>>>>>
> >>>>>> https://cwiki.apache.org/confluence/x/sYkhFg
> >>>>>>
> >>>>>> Thanks,
> >>>>>> PoAn
> >>>
> >>>
> >>
>
>

Reply via email to