Hi Divij,
I am fine with having an API to compute the size as I mentioned in my
earlier reply in this mail thread. But I have the below comment for
the motivation for this KIP.

As you discussed offline, the main issue here is listing calls for
remote log segment metadata is slower because of the storage used for
RLMM. These can be avoided with this new API.

Please add this in the motivation section as it is one of the main
motivations for the KIP.

Thanks,
Satish.

On Sat, 1 Jul 2023 at 01:43, Jun Rao <j...@confluent.io.invalid> wrote:
>
> Hi, Divij,
>
> Sorry for the late reply.
>
> Given your explanation, the new API sounds reasonable to me. Is that enough
> to build the external metadata layer for the remote segments or do you need
> some additional API changes?
>
> Thanks,
>
> Jun
>
> On Fri, Jun 9, 2023 at 7:08 AM Divij Vaidya <divijvaidy...@gmail.com> wrote:
>
> > Thank you for looking into this Kamal.
> >
> > You are right in saying that a cold start (i.e. leadership failover or
> > broker startup) does not impact the broker startup duration. But it does
> > have the following impact:
> > 1. It leads to a burst of full-scan requests to RLMM in case multiple
> > leadership failovers occur at the same time. Even if the RLMM
> > implementation has the capability to serve the total size from an index
> > (and hence handle this burst), we wouldn't be able to use it since the
> > current API necessarily calls for a full scan.
> > 2. The archival (copying of data to tiered storage) process will have a
> > delayed start. The delayed start of archival could lead to local build up
> > of data which may lead to disk full.
> >
> > The disadvantage of adding this new API is that every provider will have to
> > implement it, agreed. But I believe that this tradeoff is worthwhile since
> > the default implementation could be the same as you mentioned, i.e. keeping
> > cumulative in-memory count.
> >
> > --
> > Divij Vaidya
> >
> >
> >
> > On Sun, Jun 4, 2023 at 5:48 PM Kamal Chandraprakash <
> > kamal.chandraprak...@gmail.com> wrote:
> >
> > > Hi Divij,
> > >
> > > Thanks for the KIP! Sorry for the late reply.
> > >
> > > Can you explain the rejected alternative-3?
> > > Store the cumulative size of remote tier log in-memory at
> > RemoteLogManager
> > > "*Cons*: Every time a broker starts-up, it will scan through all the
> > > segments in the remote tier to initialise the in-memory value. This would
> > > increase the broker start-up time."
> > >
> > > Keeping the source of truth to determine the remote-log-size in the
> > leader
> > > would be consistent across different implementations of the plugin. The
> > > concern posted in the KIP is that we are calculating the remote-log-size
> > on
> > > each iteration of the cleaner thread (say 5 mins). If we calculate only
> > > once during broker startup or during the leadership reassignment, do we
> > > still need the cache?
> > >
> > > The broker startup-time won't be affected by the remote log manager
> > > initialisation. The broker continue to start accepting the new
> > > produce/fetch requests, while the RLM thread in the background can
> > > determine the remote-log-size once and start copying/deleting the
> > segments.
> > >
> > > Thanks,
> > > Kamal
> > >
> > > On Thu, Jun 1, 2023 at 2:08 PM Divij Vaidya <divijvaidy...@gmail.com>
> > > wrote:
> > >
> > > > Satish / Jun
> > > >
> > > > Do you have any thoughts on this?
> > > >
> > > > --
> > > > Divij Vaidya
> > > >
> > > >
> > > >
> > > > On Tue, Feb 14, 2023 at 4:15 PM Divij Vaidya <divijvaidy...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey Jun
> > > > >
> > > > > It has been a while since this KIP got some attention. While we wait
> > > for
> > > > > Satish to chime in here, perhaps I can answer your question.
> > > > >
> > > > > > Could you explain how you exposed the log size in your KIP-405
> > > > > implementation?
> > > > >
> > > > > The APIs available in RLMM as per KIP405
> > > > > are, addRemoteLogSegmentMetadata(), updateRemoteLogSegmentMetadata(),
> > > > remoteLogSegmentMetadata(), highestOffsetForEpoch(),
> > > > putRemotePartitionDeleteMetadata(), listRemoteLogSegments(),
> > > > onPartitionLeadershipChanges()
> > > > > and onStopPartitions(). None of these APIs allow us to expose the log
> > > > size,
> > > > > hence, the only option that remains is to list all segments using
> > > > > listRemoteLogSegments() and aggregate them every time we require to
> > > > > calculate the size. Based on our prior discussion, this requires
> > > reading
> > > > > all segment metadata which won't work for non-local RLMM
> > > implementations.
> > > > > Satish's implementation also performs a full scan and calculates the
> > > > > aggregate. see:
> > > > >
> > > >
> > >
> > https://github.com/satishd/kafka/blob/2.8.x-tiered-storage/core/src/main/scala/kafka/log/remote/RemoteLogManager.scala#L619
> > > > >
> > > > >
> > > > > Does this answer your question?
> > > > >
> > > > > --
> > > > > Divij Vaidya
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Dec 20, 2022 at 8:40 PM Jun Rao <j...@confluent.io.invalid>
> > > > wrote:
> > > > >
> > > > >> Hi, Divij,
> > > > >>
> > > > >> Thanks for the explanation.
> > > > >>
> > > > >> Good question.
> > > > >>
> > > > >> Hi, Satish,
> > > > >>
> > > > >> Could you explain how you exposed the log size in your KIP-405
> > > > >> implementation?
> > > > >>
> > > > >> Thanks,
> > > > >>
> > > > >> Jun
> > > > >>
> > > > >> On Tue, Dec 20, 2022 at 4:59 AM Divij Vaidya <
> > divijvaidy...@gmail.com
> > > >
> > > > >> wrote:
> > > > >>
> > > > >> > Hey Jun
> > > > >> >
> > > > >> > Yes, it is possible to maintain the log size in the cache (see
> > > > rejected
> > > > >> > alternative#3 in the KIP) but I did not understand how it is
> > > possible
> > > > to
> > > > >> > retrieve it without the new API. The log size could be calculated
> > on
> > > > >> > startup by scanning through the segments (though I would disagree
> > > that
> > > > >> this
> > > > >> > is the right approach since scanning itself takes order of minutes
> > > and
> > > > >> > hence delay the start of archive process), and incrementally
> > > > maintained
> > > > >> > afterwards, even then, we would need an API in
> > > > RemoteLogMetadataManager
> > > > >> so
> > > > >> > that RLM could fetch the cached size!
> > > > >> >
> > > > >> > If we wish to cache the size without adding a new API, then we
> > need
> > > to
> > > > >> > cache the size in RLM itself (instead of RLMM implementation) and
> > > > >> > incrementally manage it. The downside of longer archive time at
> > > > startup
> > > > >> > (due to initial scale) still remains valid in this situation.
> > > > >> >
> > > > >> > --
> > > > >> > Divij Vaidya
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Fri, Dec 16, 2022 at 12:43 AM Jun Rao <j...@confluent.io.invalid
> > >
> > > > >> wrote:
> > > > >> >
> > > > >> > > Hi, Divij,
> > > > >> > >
> > > > >> > > Thanks for the explanation.
> > > > >> > >
> > > > >> > > If there is in-memory cache, could we maintain the log size in
> > the
> > > > >> cache
> > > > >> > > with the existing API? For example, a replica could make a
> > > > >> > > listRemoteLogSegments(TopicIdPartition topicIdPartition) call on
> > > > >> startup
> > > > >> > to
> > > > >> > > get the remote segment size before the current leaderEpoch. The
> > > > leader
> > > > >> > > could then maintain the size incrementally afterwards. On leader
> > > > >> change,
> > > > >> > > other replicas can make a listRemoteLogSegments(TopicIdPartition
> > > > >> > > topicIdPartition, int leaderEpoch) call to get the size of newly
> > > > >> > generated
> > > > >> > > segments.
> > > > >> > >
> > > > >> > > Thanks,
> > > > >> > >
> > > > >> > > Jun
> > > > >> > >
> > > > >> > >
> > > > >> > > On Wed, Dec 14, 2022 at 3:27 AM Divij Vaidya <
> > > > divijvaidy...@gmail.com
> > > > >> >
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > > Is the new method enough for doing size-based retention?
> > > > >> > > >
> > > > >> > > > Yes. You are right in assuming that this API only provides the
> > > > >> Remote
> > > > >> > > > storage size (for current epoch chain). We would use this API
> > > for
> > > > >> size
> > > > >> > > > based retention along with a value of localOnlyLogSegmentSize
> > > > which
> > > > >> is
> > > > >> > > > computed as Log.sizeInBytes(logSegments.filter(_.baseOffset >
> > > > >> > > > highestOffsetWithRemoteIndex)). Hence, (total_log_size =
> > > > >> > > > remoteLogSizeBytes + log.localOnlyLogSegmentSize). I have
> > > updated
> > > > >> the
> > > > >> > KIP
> > > > >> > > > with this information. You can also check an example
> > > > implementation
> > > > >> at
> > > > >> > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> > https://github.com/satishd/kafka/blob/2.8.x-tiered-storage/core/src/main/scala/kafka/log/Log.scala#L2077
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > > Do you imagine all accesses to remote metadata will be
> > across
> > > > the
> > > > >> > > network
> > > > >> > > > or will there be some local in-memory cache?
> > > > >> > > >
> > > > >> > > > I would expect a disk-less implementation to maintain a finite
> > > > >> > in-memory
> > > > >> > > > cache for segment metadata to optimize the number of network
> > > calls
> > > > >> made
> > > > >> > > to
> > > > >> > > > fetch the data. In future, we can think about bringing this
> > > finite
> > > > >> size
> > > > >> > > > cache into RLM itself but that's probably a conversation for a
> > > > >> > different
> > > > >> > > > KIP. There are many other things we would like to do to
> > optimize
> > > > the
> > > > >> > > Tiered
> > > > >> > > > storage interface such as introducing a circular buffer /
> > > > streaming
> > > > >> > > > interface from RSM (so that we don't have to wait to fetch the
> > > > >> entire
> > > > >> > > > segment before starting to send records to the consumer),
> > > caching
> > > > >> the
> > > > >> > > > segments fetched from RSM locally (I would assume all RSM
> > plugin
> > > > >> > > > implementations to do this, might as well add it to RLM) etc.
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Divij Vaidya
> > > > >> > > >
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Mon, Dec 12, 2022 at 7:35 PM Jun Rao
> > > <j...@confluent.io.invalid
> > > > >
> > > > >> > > wrote:
> > > > >> > > >
> > > > >> > > > > Hi, Divij,
> > > > >> > > > >
> > > > >> > > > > Thanks for the reply.
> > > > >> > > > >
> > > > >> > > > > Is the new method enough for doing size-based retention? It
> > > > gives
> > > > >> the
> > > > >> > > > total
> > > > >> > > > > size of the remote segments, but it seems that we still
> > don't
> > > > know
> > > > >> > the
> > > > >> > > > > exact total size for a log since there could be overlapping
> > > > >> segments
> > > > >> > > > > between the remote and the local segments.
> > > > >> > > > >
> > > > >> > > > > You mentioned a disk-less implementation. Do you imagine all
> > > > >> accesses
> > > > >> > > to
> > > > >> > > > > remote metadata will be across the network or will there be
> > > some
> > > > >> > local
> > > > >> > > > > in-memory cache?
> > > > >> > > > >
> > > > >> > > > > Thanks,
> > > > >> > > > >
> > > > >> > > > > Jun
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > On Wed, Dec 7, 2022 at 3:10 AM Divij Vaidya <
> > > > >> divijvaidy...@gmail.com
> > > > >> > >
> > > > >> > > > > wrote:
> > > > >> > > > >
> > > > >> > > > > > The method is needed for RLMM implementations which fetch
> > > the
> > > > >> > > > information
> > > > >> > > > > > over the network and not for the disk based
> > implementations
> > > > >> (such
> > > > >> > as
> > > > >> > > > the
> > > > >> > > > > > default topic based RLMM).
> > > > >> > > > > >
> > > > >> > > > > > I would argue that adding this API makes the interface
> > more
> > > > >> generic
> > > > >> > > > than
> > > > >> > > > > > what it is today. This is because, with the current APIs
> > an
> > > > >> > > implementor
> > > > >> > > > > is
> > > > >> > > > > > restricted to use disk based RLMM solutions only (i.e. the
> > > > >> default
> > > > >> > > > > > solution) whereas if we add this new API, we unblock usage
> > > of
> > > > >> > network
> > > > >> > > > > based
> > > > >> > > > > > RLMM implementations such as databases.
> > > > >> > > > > >
> > > > >> > > > > >
> > > > >> > > > > >
> > > > >> > > > > > On Wed 30. Nov 2022 at 20:40, Jun Rao
> > > > <j...@confluent.io.invalid
> > > > >> >
> > > > >> > > > wrote:
> > > > >> > > > > >
> > > > >> > > > > > > Hi, Divij,
> > > > >> > > > > > >
> > > > >> > > > > > > Thanks for the reply.
> > > > >> > > > > > >
> > > > >> > > > > > > Point#2. My high level question is that is the new
> > method
> > > > >> needed
> > > > >> > > for
> > > > >> > > > > > every
> > > > >> > > > > > > implementation of remote storage or just for a specific
> > > > >> > > > implementation.
> > > > >> > > > > > The
> > > > >> > > > > > > issues that you pointed out exist for the default
> > > > >> implementation
> > > > >> > of
> > > > >> > > > > RLMM
> > > > >> > > > > > as
> > > > >> > > > > > > well and so far, the default implementation hasn't
> > found a
> > > > >> need
> > > > >> > > for a
> > > > >> > > > > > > similar new method. For public interface, ideally we
> > want
> > > to
> > > > >> make
> > > > >> > > it
> > > > >> > > > > more
> > > > >> > > > > > > general.
> > > > >> > > > > > >
> > > > >> > > > > > > Thanks,
> > > > >> > > > > > >
> > > > >> > > > > > > Jun
> > > > >> > > > > > >
> > > > >> > > > > > > On Mon, Nov 21, 2022 at 7:11 AM Divij Vaidya <
> > > > >> > > > divijvaidy...@gmail.com>
> > > > >> > > > > > > wrote:
> > > > >> > > > > > >
> > > > >> > > > > > > > Thank you Jun and Alex for your comments.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Point#1: You are right Jun. As Alex mentioned, the
> > > > "derived
> > > > >> > > > metadata"
> > > > >> > > > > > can
> > > > >> > > > > > > > increase the size of cached metadata by a factor of 10
> > > but
> > > > >> it
> > > > >> > > > should
> > > > >> > > > > be
> > > > >> > > > > > > ok
> > > > >> > > > > > > > to cache just the actual metadata. My point about size
> > > > >> being a
> > > > >> > > > > > limitation
> > > > >> > > > > > > > for using cache is not valid anymore.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Point#2: For a new replica, it would still have to
> > fetch
> > > > the
> > > > >> > > > metadata
> > > > >> > > > > > > over
> > > > >> > > > > > > > the network to initiate the warm up of the cache and
> > > > hence,
> > > > >> > > > increase
> > > > >> > > > > > the
> > > > >> > > > > > > > start time of the archival process. Please also note
> > the
> > > > >> > > > > repercussions
> > > > >> > > > > > of
> > > > >> > > > > > > > the warm up scan that Alex mentioned in this thread as
> > > > part
> > > > >> of
> > > > >> > > > > #102.2.
> > > > >> > > > > > > >
> > > > >> > > > > > > > 100#: Agreed Alex. Thanks for clarifying that. My
> > point
> > > > >> about
> > > > >> > > size
> > > > >> > > > > > being
> > > > >> > > > > > > a
> > > > >> > > > > > > > limitation for using cache is not valid anymore.
> > > > >> > > > > > > >
> > > > >> > > > > > > > 101#: Alex, if I understand correctly, you are
> > > suggesting
> > > > to
> > > > >> > > cache
> > > > >> > > > > the
> > > > >> > > > > > > > total size at the leader and update it on archival.
> > This
> > > > >> > wouldn't
> > > > >> > > > > work
> > > > >> > > > > > > for
> > > > >> > > > > > > > cases when the leader restarts where we would have to
> > > > make a
> > > > >> > full
> > > > >> > > > > scan
> > > > >> > > > > > > > to update the total size entry on startup. We expect
> > > users
> > > > >> to
> > > > >> > > store
> > > > >> > > > > > data
> > > > >> > > > > > > > over longer duration in remote storage which increases
> > > the
> > > > >> > > > likelihood
> > > > >> > > > > > of
> > > > >> > > > > > > > leader restarts / failovers.
> > > > >> > > > > > > >
> > > > >> > > > > > > > 102#.1: I don't think that the current design
> > > accommodates
> > > > >> the
> > > > >> > > fact
> > > > >> > > > > > that
> > > > >> > > > > > > > data corruption could happen at the RLMM plugin (we
> > > don't
> > > > >> have
> > > > >> > > > > checksum
> > > > >> > > > > > > as
> > > > >> > > > > > > > a field in metadata as part of KIP405). If data
> > > corruption
> > > > >> > > occurs,
> > > > >> > > > w/
> > > > >> > > > > > or
> > > > >> > > > > > > > w/o the cache, it would be a different problem to
> > > solve. I
> > > > >> > would
> > > > >> > > > like
> > > > >> > > > > > to
> > > > >> > > > > > > > keep this outside the scope of this KIP.
> > > > >> > > > > > > >
> > > > >> > > > > > > > 102#.2: Agree. This remains as the main concern for
> > > using
> > > > >> the
> > > > >> > > cache
> > > > >> > > > > to
> > > > >> > > > > > > > fetch total size.
> > > > >> > > > > > > >
> > > > >> > > > > > > > Regards,
> > > > >> > > > > > > > Divij Vaidya
> > > > >> > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > > On Fri, Nov 18, 2022 at 12:59 PM Alexandre Dupriez <
> > > > >> > > > > > > > alexandre.dupr...@gmail.com> wrote:
> > > > >> > > > > > > >
> > > > >> > > > > > > > > Hi Divij,
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > Thanks for the KIP. Please find some comments based
> > on
> > > > >> what I
> > > > >> > > > read
> > > > >> > > > > on
> > > > >> > > > > > > > > this thread so far - apologies for the repeats and
> > the
> > > > >> late
> > > > >> > > > reply.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > If I understand correctly, one of the main elements
> > of
> > > > >> > > discussion
> > > > >> > > > > is
> > > > >> > > > > > > > > about caching in Kafka versus delegation of
> > providing
> > > > the
> > > > >> > > remote
> > > > >> > > > > size
> > > > >> > > > > > > > > of a topic-partition to the plugin.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > A few comments:
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > 100. The size of the “derived metadata” which is
> > > managed
> > > > >> by
> > > > >> > the
> > > > >> > > > > > plugin
> > > > >> > > > > > > > > to represent an rlmMetadata can indeed be close to 1
> > > kB
> > > > on
> > > > >> > > > average
> > > > >> > > > > > > > > depending on its own internal structure, e.g. the
> > > > >> redundancy
> > > > >> > it
> > > > >> > > > > > > > > enforces (unfortunately resulting to duplication),
> > > > >> additional
> > > > >> > > > > > > > > information such as checksums and primary and
> > > secondary
> > > > >> > > indexable
> > > > >> > > > > > > > > keys. But indeed, the rlmMetadata is itself a
> > lighter
> > > > data
> > > > >> > > > > structure
> > > > >> > > > > > > > > by a factor of 10. And indeed, instead of caching
> > the
> > > > >> > “derived
> > > > >> > > > > > > > > metadata”, only the rlmMetadata could be, which
> > should
> > > > >> > address
> > > > >> > > > the
> > > > >> > > > > > > > > concern regarding the memory occupancy of the cache.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > 101. I am not sure I fully understand why we would
> > > need
> > > > to
> > > > >> > > cache
> > > > >> > > > > the
> > > > >> > > > > > > > > list of rlmMetadata to retain the remote size of a
> > > > >> > > > topic-partition.
> > > > >> > > > > > > > > Since the leader of a topic-partition is, in
> > > > >> non-degenerated
> > > > >> > > > cases,
> > > > >> > > > > > > > > the only actor which can mutate the remote part of
> > the
> > > > >> > > > > > > > > topic-partition, hence its size, it could in theory
> > > only
> > > > >> > cache
> > > > >> > > > the
> > > > >> > > > > > > > > size of the remote log once it has calculated it? In
> > > > which
> > > > >> > case
> > > > >> > > > > there
> > > > >> > > > > > > > > would not be any problem regarding the size of the
> > > > caching
> > > > >> > > > > strategy.
> > > > >> > > > > > > > > Did I miss something there?
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > 102. There may be a few challenges to consider with
> > > > >> caching:
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > 102.1) As mentioned above, the caching strategy
> > > assumes
> > > > no
> > > > >> > > > mutation
> > > > >> > > > > > > > > outside the lifetime of a leader. While this is true
> > > in
> > > > >> the
> > > > >> > > > normal
> > > > >> > > > > > > > > course of operation, there could be accidental
> > > mutation
> > > > >> > outside
> > > > >> > > > of
> > > > >> > > > > > the
> > > > >> > > > > > > > > leader and a loss of consistency between the cached
> > > > state
> > > > >> and
> > > > >> > > the
> > > > >> > > > > > > > > actual remote representation of the log. E.g.
> > > > split-brain
> > > > >> > > > > scenarios,
> > > > >> > > > > > > > > bugs in the plugins, bugs in external systems with
> > > > >> mutating
> > > > >> > > > access
> > > > >> > > > > on
> > > > >> > > > > > > > > the derived metadata. In the worst case, a drift
> > > between
> > > > >> the
> > > > >> > > > cached
> > > > >> > > > > > > > > size and the actual size could lead to over-deleting
> > > > >> remote
> > > > >> > > data
> > > > >> > > > > > which
> > > > >> > > > > > > > > is a durability risk.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > The alternative you propose, by making the plugin
> > the
> > > > >> source
> > > > >> > of
> > > > >> > > > > truth
> > > > >> > > > > > > > > w.r.t. to the size of the remote log, can make it
> > > easier
> > > > >> to
> > > > >> > > avoid
> > > > >> > > > > > > > > inconsistencies between plugin-managed metadata and
> > > the
> > > > >> > remote
> > > > >> > > > log
> > > > >> > > > > > > > > from the perspective of Kafka. On the other hand,
> > > plugin
> > > > >> > > vendors
> > > > >> > > > > > would
> > > > >> > > > > > > > > have to implement it with the expected efficiency to
> > > > have
> > > > >> it
> > > > >> > > > yield
> > > > >> > > > > > > > > benefits.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > 102.2) As you mentioned, the caching strategy in
> > Kafka
> > > > >> would
> > > > >> > > > still
> > > > >> > > > > > > > > require one iteration over the list of rlmMetadata
> > > when
> > > > >> the
> > > > >> > > > > > leadership
> > > > >> > > > > > > > > of a topic-partition is assigned to a broker, while
> > > the
> > > > >> > plugin
> > > > >> > > > can
> > > > >> > > > > > > > > offer alternative constant-time approaches. This
> > > > >> calculation
> > > > >> > > > cannot
> > > > >> > > > > > be
> > > > >> > > > > > > > > put on the LeaderAndIsr path and would be performed
> > in
> > > > the
> > > > >> > > > > > background.
> > > > >> > > > > > > > > In case of bulk leadership migration, listing the
> > > > >> rlmMetadata
> > > > >> > > > could
> > > > >> > > > > > a)
> > > > >> > > > > > > > > result in request bursts to any backend system the
> > > > plugin
> > > > >> may
> > > > >> > > use
> > > > >> > > > > > > > > [which shouldn’t be a problem for high-throughput
> > data
> > > > >> stores
> > > > >> > > but
> > > > >> > > > > > > > > could have cost implications] b) increase
> > utilisation
> > > > >> > timespan
> > > > >> > > of
> > > > >> > > > > the
> > > > >> > > > > > > > > RLM threads for these calculations potentially
> > leading
> > > > to
> > > > >> > > > transient
> > > > >> > > > > > > > > starvation of tasks queued for, typically,
> > offloading
> > > > >> > > operations
> > > > >> > > > c)
> > > > >> > > > > > > > > could have a non-marginal CPU footprint on hardware
> > > with
> > > > >> > strict
> > > > >> > > > > > > > > resource constraints. All these elements could have
> > an
> > > > >> impact
> > > > >> > > to
> > > > >> > > > > some
> > > > >> > > > > > > > > degree depending on the operational environment.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > From a design perspective, one question is where we
> > > want
> > > > >> the
> > > > >> > > > source
> > > > >> > > > > > of
> > > > >> > > > > > > > > truth w.r.t. remote log size to be during the
> > lifetime
> > > > of
> > > > >> a
> > > > >> > > > leader.
> > > > >> > > > > > > > > The responsibility of maintaining a consistent
> > > > >> representation
> > > > >> > > of
> > > > >> > > > > the
> > > > >> > > > > > > > > remote log is shared by Kafka and the plugin. Which
> > > > >> system is
> > > > >> > > > best
> > > > >> > > > > > > > > placed to maintain such a state while providing the
> > > > >> highest
> > > > >> > > > > > > > > consistency guarantees is something both Kafka and
> > > > plugin
> > > > >> > > > designers
> > > > >> > > > > > > > > could help understand better.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > Many thanks,
> > > > >> > > > > > > > > Alexandre
> > > > >> > > > > > > > >
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > Le jeu. 17 nov. 2022 à 19:27, Jun Rao
> > > > >> > <j...@confluent.io.invalid
> > > > >> > > >
> > > > >> > > > a
> > > > >> > > > > > > > écrit :
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Hi, Divij,
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Thanks for the reply.
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Point #1. Is the average remote segment metadata
> > > > really
> > > > >> > 1KB?
> > > > >> > > > > What's
> > > > >> > > > > > > > > listed
> > > > >> > > > > > > > > > in the public interface is probably well below 100
> > > > >> bytes.
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Point #2. I guess you are assuming that each
> > broker
> > > > only
> > > > >> > > caches
> > > > >> > > > > the
> > > > >> > > > > > > > > remote
> > > > >> > > > > > > > > > segment metadata in memory. An alternative
> > approach
> > > is
> > > > >> to
> > > > >> > > cache
> > > > >> > > > > > them
> > > > >> > > > > > > in
> > > > >> > > > > > > > > > both memory and local disk. That way, on broker
> > > > restart,
> > > > >> > you
> > > > >> > > > just
> > > > >> > > > > > > need
> > > > >> > > > > > > > to
> > > > >> > > > > > > > > > fetch the new remote segments' metadata using the
> > > > >> > > > > > > > > > listRemoteLogSegments(TopicIdPartition
> > > > topicIdPartition,
> > > > >> > int
> > > > >> > > > > > > > leaderEpoch)
> > > > >> > > > > > > > > > api. Will that work?
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Point #3. Thanks for the explanation and it sounds
> > > > good.
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Thanks,
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > Jun
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > On Thu, Nov 17, 2022 at 7:31 AM Divij Vaidya <
> > > > >> > > > > > > divijvaidy...@gmail.com>
> > > > >> > > > > > > > > > wrote:
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > > Hi Jun
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > There are three points that I would like to
> > > present
> > > > >> here:
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > 1. We would require a large cache size to
> > > > efficiently
> > > > >> > cache
> > > > >> > > > all
> > > > >> > > > > > > > segment
> > > > >> > > > > > > > > > > metadata.
> > > > >> > > > > > > > > > > 2. Linear scan of all metadata at broker startup
> > > to
> > > > >> > > populate
> > > > >> > > > > the
> > > > >> > > > > > > > cache
> > > > >> > > > > > > > > will
> > > > >> > > > > > > > > > > be slow and will impact the archival process.
> > > > >> > > > > > > > > > > 3. There is no other use case where a full scan
> > of
> > > > >> > segment
> > > > >> > > > > > metadata
> > > > >> > > > > > > > is
> > > > >> > > > > > > > > > > required.
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > Let's start by quantifying 1. Here's my estimate
> > > for
> > > > >> the
> > > > >> > > size
> > > > >> > > > > of
> > > > >> > > > > > > the
> > > > >> > > > > > > > > cache.
> > > > >> > > > > > > > > > > Average size of segment metadata = 1KB. This
> > could
> > > > be
> > > > >> > more
> > > > >> > > if
> > > > >> > > > > we
> > > > >> > > > > > > have
> > > > >> > > > > > > > > > > frequent leader failover with a large number of
> > > > leader
> > > > >> > > epochs
> > > > >> > > > > > being
> > > > >> > > > > > > > > stored
> > > > >> > > > > > > > > > > per segment.
> > > > >> > > > > > > > > > > Segment size = 100MB. Users will prefer to
> > reduce
> > > > the
> > > > >> > > segment
> > > > >> > > > > > size
> > > > >> > > > > > > > > from the
> > > > >> > > > > > > > > > > default value of 1GB to ensure timely archival
> > of
> > > > data
> > > > >> > > since
> > > > >> > > > > data
> > > > >> > > > > > > > from
> > > > >> > > > > > > > > > > active segment is not archived.
> > > > >> > > > > > > > > > > Cache size = num segments * avg. segment
> > metadata
> > > > >> size =
> > > > >> > > > > > > > > (100TB/100MB)*1KB
> > > > >> > > > > > > > > > > = 1GB.
> > > > >> > > > > > > > > > > While 1GB for cache may not sound like a large
> > > > number
> > > > >> for
> > > > >> > > > > larger
> > > > >> > > > > > > > > machines,
> > > > >> > > > > > > > > > > it does eat into the memory as an additional
> > cache
> > > > and
> > > > >> > > makes
> > > > >> > > > > use
> > > > >> > > > > > > > cases
> > > > >> > > > > > > > > with
> > > > >> > > > > > > > > > > large data retention with low throughout
> > expensive
> > > > >> (where
> > > > >> > > > such
> > > > >> > > > > > use
> > > > >> > > > > > > > case
> > > > >> > > > > > > > > > > would could use smaller machines).
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > About point#2:
> > > > >> > > > > > > > > > > Even if we say that all segment metadata can fit
> > > > into
> > > > >> the
> > > > >> > > > > cache,
> > > > >> > > > > > we
> > > > >> > > > > > > > > will
> > > > >> > > > > > > > > > > need to populate the cache on broker startup. It
> > > > would
> > > > >> > not
> > > > >> > > be
> > > > >> > > > > in
> > > > >> > > > > > > the
> > > > >> > > > > > > > > > > critical patch of broker startup and hence won't
> > > > >> impact
> > > > >> > the
> > > > >> > > > > > startup
> > > > >> > > > > > > > > time.
> > > > >> > > > > > > > > > > But it will impact the time when we could start
> > > the
> > > > >> > > archival
> > > > >> > > > > > > process
> > > > >> > > > > > > > > since
> > > > >> > > > > > > > > > > the RLM thread pool will be blocked on the first
> > > > call
> > > > >> to
> > > > >> > > > > > > > > > > listRemoteLogSegments(). To scan metadata for
> > 1MM
> > > > >> > segments
> > > > >> > > > > > > (computed
> > > > >> > > > > > > > > above)
> > > > >> > > > > > > > > > > and transfer 1GB data over the network from a
> > RLMM
> > > > >> such
> > > > >> > as
> > > > >> > > a
> > > > >> > > > > > remote
> > > > >> > > > > > > > > > > database would be in the order of minutes
> > > (depending
> > > > >> on
> > > > >> > how
> > > > >> > > > > > > efficient
> > > > >> > > > > > > > > the
> > > > >> > > > > > > > > > > scan is with the RLMM implementation).
> > Although, I
> > > > >> would
> > > > >> > > > > concede
> > > > >> > > > > > > that
> > > > >> > > > > > > > > > > having RLM threads blocked for a few minutes is
> > > > >> perhaps
> > > > >> > OK
> > > > >> > > > but
> > > > >> > > > > if
> > > > >> > > > > > > we
> > > > >> > > > > > > > > > > introduce the new API proposed in the KIP, we
> > > would
> > > > >> have
> > > > >> > a
> > > > >> > > > > > > > > > > deterministic startup time for RLM. Adding the
> > API
> > > > >> comes
> > > > >> > > at a
> > > > >> > > > > low
> > > > >> > > > > > > > cost
> > > > >> > > > > > > > > and
> > > > >> > > > > > > > > > > I believe the trade off is worth it.
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > About point#3:
> > > > >> > > > > > > > > > > We can use
> > listRemoteLogSegments(TopicIdPartition
> > > > >> > > > > > topicIdPartition,
> > > > >> > > > > > > > int
> > > > >> > > > > > > > > > > leaderEpoch) to calculate the segments eligible
> > > for
> > > > >> > > deletion
> > > > >> > > > > > (based
> > > > >> > > > > > > > on
> > > > >> > > > > > > > > size
> > > > >> > > > > > > > > > > retention) where leader epoch(s) belong to the
> > > > current
> > > > >> > > leader
> > > > >> > > > > > epoch
> > > > >> > > > > > > > > chain.
> > > > >> > > > > > > > > > > I understand that it may lead to segments
> > > belonging
> > > > to
> > > > >> > > other
> > > > >> > > > > > epoch
> > > > >> > > > > > > > > lineage
> > > > >> > > > > > > > > > > not getting deleted and would require a separate
> > > > >> > mechanism
> > > > >> > > to
> > > > >> > > > > > > delete
> > > > >> > > > > > > > > them.
> > > > >> > > > > > > > > > > The separate mechanism would anyways be required
> > > to
> > > > >> > delete
> > > > >> > > > > these
> > > > >> > > > > > > > > "leaked"
> > > > >> > > > > > > > > > > segments as there are other cases which could
> > lead
> > > > to
> > > > >> > leaks
> > > > >> > > > > such
> > > > >> > > > > > as
> > > > >> > > > > > > > > network
> > > > >> > > > > > > > > > > problems with RSM mid way writing through.
> > segment
> > > > >> etc.
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > Thank you for the replies so far. They have made
> > > me
> > > > >> > > re-think
> > > > >> > > > my
> > > > >> > > > > > > > > assumptions
> > > > >> > > > > > > > > > > and this dialogue has been very constructive for
> > > me.
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > Regards,
> > > > >> > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > On Thu, Nov 10, 2022 at 10:49 PM Jun Rao
> > > > >> > > > > > <j...@confluent.io.invalid
> > > > >> > > > > > > >
> > > > >> > > > > > > > > wrote:
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > > Hi, Divij,
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Thanks for the reply.
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > It's true that the data in Kafka could be kept
> > > > >> longer
> > > > >> > > with
> > > > >> > > > > > > KIP-405.
> > > > >> > > > > > > > > How
> > > > >> > > > > > > > > > > > much data do you envision to have per broker?
> > > For
> > > > >> 100TB
> > > > >> > > > data
> > > > >> > > > > > per
> > > > >> > > > > > > > > broker,
> > > > >> > > > > > > > > > > > with 1GB segment and segment metadata of 100
> > > > bytes,
> > > > >> it
> > > > >> > > > > requires
> > > > >> > > > > > > > > > > > 100TB/1GB*100 = 10MB, which should fit in
> > > memory.
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > RemoteLogMetadataManager has two
> > > > >> > listRemoteLogSegments()
> > > > >> > > > > > methods.
> > > > >> > > > > > > > > The one
> > > > >> > > > > > > > > > > > you listed
> > > listRemoteLogSegments(TopicIdPartition
> > > > >> > > > > > > topicIdPartition,
> > > > >> > > > > > > > > int
> > > > >> > > > > > > > > > > > leaderEpoch) does return data in offset order.
> > > > >> However,
> > > > >> > > the
> > > > >> > > > > > other
> > > > >> > > > > > > > > > > > one listRemoteLogSegments(TopicIdPartition
> > > > >> > > > topicIdPartition)
> > > > >> > > > > > > > doesn't
> > > > >> > > > > > > > > > > > specify the return order. I assume that you
> > need
> > > > the
> > > > >> > > latter
> > > > >> > > > > to
> > > > >> > > > > > > > > calculate
> > > > >> > > > > > > > > > > > the segment size?
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Thanks,
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Jun
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > On Thu, Nov 10, 2022 at 10:25 AM Divij Vaidya
> > <
> > > > >> > > > > > > > > divijvaidy...@gmail.com>
> > > > >> > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > *Jun,*
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > *"the default implementation of RLMM does
> > > local
> > > > >> > > caching,
> > > > >> > > > > > > right?"*
> > > > >> > > > > > > > > > > > > Yes, Jun. The default implementation of RLMM
> > > > does
> > > > >> > > indeed
> > > > >> > > > > > cache
> > > > >> > > > > > > > the
> > > > >> > > > > > > > > > > > segment
> > > > >> > > > > > > > > > > > > metadata today, hence, it won't work for use
> > > > cases
> > > > >> > when
> > > > >> > > > the
> > > > >> > > > > > > > number
> > > > >> > > > > > > > > of
> > > > >> > > > > > > > > > > > > segments in remote storage is large enough
> > to
> > > > >> exceed
> > > > >> > > the
> > > > >> > > > > size
> > > > >> > > > > > > of
> > > > >> > > > > > > > > cache.
> > > > >> > > > > > > > > > > > As
> > > > >> > > > > > > > > > > > > part of this KIP, I will implement the new
> > > > >> proposed
> > > > >> > API
> > > > >> > > > in
> > > > >> > > > > > the
> > > > >> > > > > > > > > default
> > > > >> > > > > > > > > > > > > implementation of RLMM but the underlying
> > > > >> > > implementation
> > > > >> > > > > will
> > > > >> > > > > > > > > still be
> > > > >> > > > > > > > > > > a
> > > > >> > > > > > > > > > > > > scan. I will pick up optimizing that in a
> > > > separate
> > > > >> > PR.
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > *"we also cache all segment metadata in the
> > > > >> brokers
> > > > >> > > > without
> > > > >> > > > > > > > > KIP-405. Do
> > > > >> > > > > > > > > > > > you
> > > > >> > > > > > > > > > > > > see a need to change that?"*
> > > > >> > > > > > > > > > > > > Please correct me if I am wrong here but we
> > > > cache
> > > > >> > > > metadata
> > > > >> > > > > > for
> > > > >> > > > > > > > > segments
> > > > >> > > > > > > > > > > > > "residing in local storage". The size of the
> > > > >> current
> > > > >> > > > cache
> > > > >> > > > > > > works
> > > > >> > > > > > > > > fine
> > > > >> > > > > > > > > > > for
> > > > >> > > > > > > > > > > > > the scale of the number of segments that we
> > > > >> expect to
> > > > >> > > > store
> > > > >> > > > > > in
> > > > >> > > > > > > > > local
> > > > >> > > > > > > > > > > > > storage. After KIP-405, that cache will
> > > continue
> > > > >> to
> > > > >> > > store
> > > > >> > > > > > > > metadata
> > > > >> > > > > > > > > for
> > > > >> > > > > > > > > > > > > segments which are residing in local storage
> > > and
> > > > >> > hence,
> > > > >> > > > we
> > > > >> > > > > > > don't
> > > > >> > > > > > > > > need
> > > > >> > > > > > > > > > > to
> > > > >> > > > > > > > > > > > > change that. For segments which have been
> > > > >> offloaded
> > > > >> > to
> > > > >> > > > > remote
> > > > >> > > > > > > > > storage,
> > > > >> > > > > > > > > > > it
> > > > >> > > > > > > > > > > > > would rely on RLMM. Note that the scale of
> > > data
> > > > >> > stored
> > > > >> > > in
> > > > >> > > > > > RLMM
> > > > >> > > > > > > is
> > > > >> > > > > > > > > > > > different
> > > > >> > > > > > > > > > > > > from local cache because the number of
> > > segments
> > > > is
> > > > >> > > > expected
> > > > >> > > > > > to
> > > > >> > > > > > > be
> > > > >> > > > > > > > > much
> > > > >> > > > > > > > > > > > > larger than what current implementation
> > stores
> > > > in
> > > > >> > local
> > > > >> > > > > > > storage.
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > 2,3,4:
> > > > >> > RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > does
> > > > >> > > > > > > > > specify
> > > > >> > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > order i.e. it returns the segments sorted by
> > > > first
> > > > >> > > offset
> > > > >> > > > > in
> > > > >> > > > > > > > > ascending
> > > > >> > > > > > > > > > > > > order. I am copying the API docs for KIP-405
> > > > here
> > > > >> for
> > > > >> > > > your
> > > > >> > > > > > > > > reference
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > *Returns iterator of remote log segment
> > > > metadata,
> > > > >> > > sorted
> > > > >> > > > by
> > > > >> > > > > > > > {@link
> > > > >> > > > > > > > > > > > > RemoteLogSegmentMetadata#startOffset()}
> > > > >> inascending
> > > > >> > > order
> > > > >> > > > > > which
> > > > >> > > > > > > > > > > contains
> > > > >> > > > > > > > > > > > > the given leader epoch. This is used by
> > remote
> > > > log
> > > > >> > > > > retention
> > > > >> > > > > > > > > management
> > > > >> > > > > > > > > > > > > subsystemto fetch the segment metadata for a
> > > > given
> > > > >> > > leader
> > > > >> > > > > > > > > epoch.@param
> > > > >> > > > > > > > > > > > > topicIdPartition topic partition@param
> > > > >> leaderEpoch
> > > > >> > > > > > leader
> > > > >> > > > > > > > > > > > > epoch@return
> > > > >> > > > > > > > > > > > > Iterator of remote segments, sorted by start
> > > > >> offset
> > > > >> > in
> > > > >> > > > > > > ascending
> > > > >> > > > > > > > > > > order. *
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > *Luke,*
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > 5. Note that we are trying to optimize the
> > > > >> efficiency
> > > > >> > > of
> > > > >> > > > > size
> > > > >> > > > > > > > based
> > > > >> > > > > > > > > > > > > retention for remote storage. KIP-405 does
> > not
> > > > >> > > introduce
> > > > >> > > > a
> > > > >> > > > > > new
> > > > >> > > > > > > > > config
> > > > >> > > > > > > > > > > for
> > > > >> > > > > > > > > > > > > periodically checking remote similar to
> > > > >> > > > > > > > > > > log.retention.check.interval.ms
> > > > >> > > > > > > > > > > > > which is applicable for remote storage.
> > Hence,
> > > > the
> > > > >> > > metric
> > > > >> > > > > > will
> > > > >> > > > > > > be
> > > > >> > > > > > > > > > > updated
> > > > >> > > > > > > > > > > > > at the time of invoking log retention check
> > > for
> > > > >> > remote
> > > > >> > > > tier
> > > > >> > > > > > > which
> > > > >> > > > > > > > > is
> > > > >> > > > > > > > > > > > > pending implementation today. We can perhaps
> > > > come
> > > > >> > back
> > > > >> > > > and
> > > > >> > > > > > > update
> > > > >> > > > > > > > > the
> > > > >> > > > > > > > > > > > > metric description after the implementation
> > of
> > > > log
> > > > >> > > > > retention
> > > > >> > > > > > > > check
> > > > >> > > > > > > > > in
> > > > >> > > > > > > > > > > > > RemoteLogManager.
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > --
> > > > >> > > > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > On Thu, Nov 10, 2022 at 6:16 AM Luke Chen <
> > > > >> > > > > show...@gmail.com
> > > > >> > > > > > >
> > > > >> > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > Hi Divij,
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > One more question about the metric:
> > > > >> > > > > > > > > > > > > > I think the metric will be updated when
> > > > >> > > > > > > > > > > > > > (1) each time we run the log retention
> > check
> > > > >> (that
> > > > >> > > is,
> > > > >> > > > > > > > > > > > > > log.retention.check.interval.ms)
> > > > >> > > > > > > > > > > > > > (2) When user explicitly call
> > > getRemoteLogSize
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > Is that correct?
> > > > >> > > > > > > > > > > > > > Maybe we should add a note in metric
> > > > >> description,
> > > > >> > > > > > otherwise,
> > > > >> > > > > > > > when
> > > > >> > > > > > > > > > > user
> > > > >> > > > > > > > > > > > > got,
> > > > >> > > > > > > > > > > > > > let's say 0 of RemoteLogSizeBytes, will be
> > > > >> > surprised.
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > Otherwise, LGTM
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > Thank you for the KIP
> > > > >> > > > > > > > > > > > > > Luke
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > On Thu, Nov 10, 2022 at 2:55 AM Jun Rao
> > > > >> > > > > > > > <j...@confluent.io.invalid
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > Hi, Divij,
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > Thanks for the explanation.
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > 1. Hmm, the default implementation of
> > RLMM
> > > > >> does
> > > > >> > > local
> > > > >> > > > > > > > caching,
> > > > >> > > > > > > > > > > right?
> > > > >> > > > > > > > > > > > > > > Currently, we also cache all segment
> > > > metadata
> > > > >> in
> > > > >> > > the
> > > > >> > > > > > > brokers
> > > > >> > > > > > > > > > > without
> > > > >> > > > > > > > > > > > > > > KIP-405. Do you see a need to change
> > that?
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > 2,3,4: Yes, your explanation makes
> > sense.
> > > > >> > However,
> > > > >> > > > > > > > > > > > > > > currently,
> > > > >> > > > > > RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > > > > > doesn't
> > > > >> > > > > > > > > > > > > > specify
> > > > >> > > > > > > > > > > > > > > a particular order of the iterator. Do
> > you
> > > > >> intend
> > > > >> > > to
> > > > >> > > > > > change
> > > > >> > > > > > > > > that?
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > Thanks,
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > Jun
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > On Tue, Nov 8, 2022 at 3:31 AM Divij
> > > Vaidya
> > > > <
> > > > >> > > > > > > > > > > divijvaidy...@gmail.com
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > Hey Jun
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > Thank you for your comments.
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > *1. "RLMM implementor could ensure
> > that
> > > > >> > > > > > > > > listRemoteLogSegments()
> > > > >> > > > > > > > > > > is
> > > > >> > > > > > > > > > > > > > fast"*
> > > > >> > > > > > > > > > > > > > > > This would be ideal but pragmatically,
> > > it
> > > > is
> > > > >> > > > > difficult
> > > > >> > > > > > to
> > > > >> > > > > > > > > ensure
> > > > >> > > > > > > > > > > > that
> > > > >> > > > > > > > > > > > > > > > listRemoteLogSegments() is fast. This
> > is
> > > > >> > because
> > > > >> > > of
> > > > >> > > > > the
> > > > >> > > > > > > > > > > possibility
> > > > >> > > > > > > > > > > > > of
> > > > >> > > > > > > > > > > > > > a
> > > > >> > > > > > > > > > > > > > > > large number of segments (much larger
> > > than
> > > > >> what
> > > > >> > > > Kafka
> > > > >> > > > > > > > > currently
> > > > >> > > > > > > > > > > > > handles
> > > > >> > > > > > > > > > > > > > > > with local storage today) would make
> > it
> > > > >> > > infeasible
> > > > >> > > > to
> > > > >> > > > > > > adopt
> > > > >> > > > > > > > > > > > > strategies
> > > > >> > > > > > > > > > > > > > > such
> > > > >> > > > > > > > > > > > > > > > as local caching to improve the
> > > > performance
> > > > >> of
> > > > >> > > > > > > > > > > > listRemoteLogSegments.
> > > > >> > > > > > > > > > > > > > > Apart
> > > > >> > > > > > > > > > > > > > > > from caching (which won't work due to
> > > size
> > > > >> > > > > > limitations) I
> > > > >> > > > > > > > > can't
> > > > >> > > > > > > > > > > > think
> > > > >> > > > > > > > > > > > > > of
> > > > >> > > > > > > > > > > > > > > > other strategies which may eliminate
> > the
> > > > >> need
> > > > >> > for
> > > > >> > > > IO
> > > > >> > > > > > > > > > > > > > > > operations proportional to the number
> > of
> > > > >> total
> > > > >> > > > > > segments.
> > > > >> > > > > > > > > Please
> > > > >> > > > > > > > > > > > > advise
> > > > >> > > > > > > > > > > > > > if
> > > > >> > > > > > > > > > > > > > > > you have something in mind.
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > 2.  "*If the size exceeds the
> > retention
> > > > >> size,
> > > > >> > we
> > > > >> > > > need
> > > > >> > > > > > to
> > > > >> > > > > > > > > > > determine
> > > > >> > > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > > subset of segments to delete to bring
> > > the
> > > > >> size
> > > > >> > > > within
> > > > >> > > > > > the
> > > > >> > > > > > > > > > > retention
> > > > >> > > > > > > > > > > > > > > limit.
> > > > >> > > > > > > > > > > > > > > > Do we need to call
> > > > >> > > > > > > > > > > RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > > > > > > > > > to
> > > > >> > > > > > > > > > > > > > > > determine that?"*
> > > > >> > > > > > > > > > > > > > > > Yes, we need to call
> > > > >> listRemoteLogSegments() to
> > > > >> > > > > > determine
> > > > >> > > > > > > > > which
> > > > >> > > > > > > > > > > > > > segments
> > > > >> > > > > > > > > > > > > > > > should be deleted. But there is a
> > > > difference
> > > > >> > with
> > > > >> > > > the
> > > > >> > > > > > use
> > > > >> > > > > > > > > case we
> > > > >> > > > > > > > > > > > are
> > > > >> > > > > > > > > > > > > > > > trying to optimize with this KIP. To
> > > > >> determine
> > > > >> > > the
> > > > >> > > > > > subset
> > > > >> > > > > > > > of
> > > > >> > > > > > > > > > > > segments
> > > > >> > > > > > > > > > > > > > > which
> > > > >> > > > > > > > > > > > > > > > would be deleted, we only read
> > metadata
> > > > for
> > > > >> > > > segments
> > > > >> > > > > > > which
> > > > >> > > > > > > > > would
> > > > >> > > > > > > > > > > be
> > > > >> > > > > > > > > > > > > > > deleted
> > > > >> > > > > > > > > > > > > > > > via the listRemoteLogSegments(). But
> > to
> > > > >> > determine
> > > > >> > > > the
> > > > >> > > > > > > > > > > totalLogSize,
> > > > >> > > > > > > > > > > > > > which
> > > > >> > > > > > > > > > > > > > > > is required every time retention logic
> > > > >> based on
> > > > >> > > > size
> > > > >> > > > > > > > > executes, we
> > > > >> > > > > > > > > > > > > read
> > > > >> > > > > > > > > > > > > > > > metadata of *all* the segments in
> > remote
> > > > >> > storage.
> > > > >> > > > > > Hence,
> > > > >> > > > > > > > the
> > > > >> > > > > > > > > > > number
> > > > >> > > > > > > > > > > > > of
> > > > >> > > > > > > > > > > > > > > > results returned by
> > > > >> > > > > > > > > > > >
> > > *RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > > > > > > > > > > *is
> > > > >> > > > > > > > > > > > > > > > different when we are calculating
> > > > >> totalLogSize
> > > > >> > > vs.
> > > > >> > > > > when
> > > > >> > > > > > > we
> > > > >> > > > > > > > > are
> > > > >> > > > > > > > > > > > > > > determining
> > > > >> > > > > > > > > > > > > > > > the subset of segments to delete.
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > 3.
> > > > >> > > > > > > > > > > > > > > > *"Also, what about time-based
> > retention?
> > > > To
> > > > >> > make
> > > > >> > > > that
> > > > >> > > > > > > > > efficient,
> > > > >> > > > > > > > > > > do
> > > > >> > > > > > > > > > > > > we
> > > > >> > > > > > > > > > > > > > > need
> > > > >> > > > > > > > > > > > > > > > to make some additional interface
> > > > >> changes?"*No.
> > > > >> > > > Note
> > > > >> > > > > > that
> > > > >> > > > > > > > > time
> > > > >> > > > > > > > > > > > > > complexity
> > > > >> > > > > > > > > > > > > > > > to determine the segments for
> > retention
> > > is
> > > > >> > > > different
> > > > >> > > > > > for
> > > > >> > > > > > > > time
> > > > >> > > > > > > > > > > based
> > > > >> > > > > > > > > > > > > vs.
> > > > >> > > > > > > > > > > > > > > > size based. For time based, the time
> > > > >> complexity
> > > > >> > > is
> > > > >> > > > a
> > > > >> > > > > > > > > function of
> > > > >> > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > number
> > > > >> > > > > > > > > > > > > > > > of segments which are "eligible for
> > > > >> deletion"
> > > > >> > > > (since
> > > > >> > > > > we
> > > > >> > > > > > > > only
> > > > >> > > > > > > > > read
> > > > >> > > > > > > > > > > > > > > metadata
> > > > >> > > > > > > > > > > > > > > > for segments which would be deleted)
> > > > >> whereas in
> > > > >> > > > size
> > > > >> > > > > > > based
> > > > >> > > > > > > > > > > > retention,
> > > > >> > > > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > > time complexity is a function of "all
> > > > >> segments"
> > > > >> > > > > > available
> > > > >> > > > > > > > in
> > > > >> > > > > > > > > > > remote
> > > > >> > > > > > > > > > > > > > > storage
> > > > >> > > > > > > > > > > > > > > > (metadata of all segments needs to be
> > > read
> > > > >> to
> > > > >> > > > > calculate
> > > > >> > > > > > > the
> > > > >> > > > > > > > > total
> > > > >> > > > > > > > > > > > > > size).
> > > > >> > > > > > > > > > > > > > > As
> > > > >> > > > > > > > > > > > > > > > you may observe, this KIP will bring
> > the
> > > > >> time
> > > > >> > > > > > complexity
> > > > >> > > > > > > > for
> > > > >> > > > > > > > > both
> > > > >> > > > > > > > > > > > > time
> > > > >> > > > > > > > > > > > > > > > based retention & size based retention
> > > to
> > > > >> the
> > > > >> > > same
> > > > >> > > > > > > > function.
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > 4. Also, please note that this new API
> > > > >> > introduced
> > > > >> > > > in
> > > > >> > > > > > this
> > > > >> > > > > > > > KIP
> > > > >> > > > > > > > > > > also
> > > > >> > > > > > > > > > > > > > > enables
> > > > >> > > > > > > > > > > > > > > > us to provide a metric for total size
> > of
> > > > >> data
> > > > >> > > > stored
> > > > >> > > > > in
> > > > >> > > > > > > > > remote
> > > > >> > > > > > > > > > > > > storage.
> > > > >> > > > > > > > > > > > > > > > Without the API, calculation of this
> > > > metric
> > > > >> > will
> > > > >> > > > > become
> > > > >> > > > > > > > very
> > > > >> > > > > > > > > > > > > expensive
> > > > >> > > > > > > > > > > > > > > with
> > > > >> > > > > > > > > > > > > > > > *listRemoteLogSegments().*
> > > > >> > > > > > > > > > > > > > > > I understand that your motivation here
> > > is
> > > > to
> > > > >> > > avoid
> > > > >> > > > > > > > polluting
> > > > >> > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > interface
> > > > >> > > > > > > > > > > > > > > > with optimization specific APIs and I
> > > will
> > > > >> > agree
> > > > >> > > > with
> > > > >> > > > > > > that
> > > > >> > > > > > > > > goal.
> > > > >> > > > > > > > > > > > But
> > > > >> > > > > > > > > > > > > I
> > > > >> > > > > > > > > > > > > > > > believe that this new API proposed in
> > > the
> > > > >> KIP
> > > > >> > > > brings
> > > > >> > > > > in
> > > > >> > > > > > > > > > > significant
> > > > >> > > > > > > > > > > > > > > > improvement and there is no other work
> > > > >> around
> > > > >> > > > > available
> > > > >> > > > > > > to
> > > > >> > > > > > > > > > > achieve
> > > > >> > > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > same
> > > > >> > > > > > > > > > > > > > > > performance.
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > Regards,
> > > > >> > > > > > > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > On Tue, Nov 8, 2022 at 12:12 AM Jun
> > Rao
> > > > >> > > > > > > > > <j...@confluent.io.invalid
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > Hi, Divij,
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > Thanks for the KIP. Sorry for the
> > late
> > > > >> reply.
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > The motivation of the KIP is to
> > > improve
> > > > >> the
> > > > >> > > > > > efficiency
> > > > >> > > > > > > of
> > > > >> > > > > > > > > size
> > > > >> > > > > > > > > > > > > based
> > > > >> > > > > > > > > > > > > > > > > retention. I am not sure the
> > proposed
> > > > >> changes
> > > > >> > > are
> > > > >> > > > > > > enough.
> > > > >> > > > > > > > > For
> > > > >> > > > > > > > > > > > > > example,
> > > > >> > > > > > > > > > > > > > > if
> > > > >> > > > > > > > > > > > > > > > > the size exceeds the retention size,
> > > we
> > > > >> need
> > > > >> > to
> > > > >> > > > > > > determine
> > > > >> > > > > > > > > the
> > > > >> > > > > > > > > > > > > subset
> > > > >> > > > > > > > > > > > > > of
> > > > >> > > > > > > > > > > > > > > > > segments to delete to bring the size
> > > > >> within
> > > > >> > the
> > > > >> > > > > > > retention
> > > > >> > > > > > > > > > > limit.
> > > > >> > > > > > > > > > > > Do
> > > > >> > > > > > > > > > > > > > we
> > > > >> > > > > > > > > > > > > > > > need
> > > > >> > > > > > > > > > > > > > > > > to call
> > > > >> > > > > > > RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > > > > to
> > > > >> > > > > > > > > > > > > determine
> > > > >> > > > > > > > > > > > > > > > that?
> > > > >> > > > > > > > > > > > > > > > > Also, what about time-based
> > retention?
> > > > To
> > > > >> > make
> > > > >> > > > that
> > > > >> > > > > > > > > efficient,
> > > > >> > > > > > > > > > > do
> > > > >> > > > > > > > > > > > > we
> > > > >> > > > > > > > > > > > > > > need
> > > > >> > > > > > > > > > > > > > > > > to make some additional interface
> > > > changes?
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > An alternative approach is for the
> > > RLMM
> > > > >> > > > implementor
> > > > >> > > > > > to
> > > > >> > > > > > > > make
> > > > >> > > > > > > > > > > sure
> > > > >> > > > > > > > > > > > > > > > > that
> > > > >> > > > > RemoteLogMetadataManager.listRemoteLogSegments()
> > > > >> > > > > > > is
> > > > >> > > > > > > > > fast
> > > > >> > > > > > > > > > > > > (e.g.,
> > > > >> > > > > > > > > > > > > > > with
> > > > >> > > > > > > > > > > > > > > > > local caching). This way, we could
> > > keep
> > > > >> the
> > > > >> > > > > interface
> > > > >> > > > > > > > > simple.
> > > > >> > > > > > > > > > > > Have
> > > > >> > > > > > > > > > > > > we
> > > > >> > > > > > > > > > > > > > > > > considered that?
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > Thanks,
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > Jun
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > On Wed, Sep 28, 2022 at 6:28 AM
> > Divij
> > > > >> Vaidya
> > > > >> > <
> > > > >> > > > > > > > > > > > > > divijvaidy...@gmail.com>
> > > > >> > > > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > Hey folks
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > Does anyone else have any thoughts
> > > on
> > > > >> this
> > > > >> > > > > before I
> > > > >> > > > > > > > > propose
> > > > >> > > > > > > > > > > > this
> > > > >> > > > > > > > > > > > > > for
> > > > >> > > > > > > > > > > > > > > a
> > > > >> > > > > > > > > > > > > > > > > > vote?
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > --
> > > > >> > > > > > > > > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > On Mon, Sep 5, 2022 at 12:57 PM
> > > Satish
> > > > >> > > Duggana
> > > > >> > > > <
> > > > >> > > > > > > > > > > > > > > > satish.dugg...@gmail.com
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > Thanks for the KIP Divij!
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > This is a nice improvement to
> > > avoid
> > > > >> > > > > recalculation
> > > > >> > > > > > > of
> > > > >> > > > > > > > > size.
> > > > >> > > > > > > > > > > > > > > Customized
> > > > >> > > > > > > > > > > > > > > > > > RLMMs
> > > > >> > > > > > > > > > > > > > > > > > > can implement the best possible
> > > > >> approach
> > > > >> > by
> > > > >> > > > > > caching
> > > > >> > > > > > > > or
> > > > >> > > > > > > > > > > > > > maintaining
> > > > >> > > > > > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > > > > size
> > > > >> > > > > > > > > > > > > > > > > > > in an efficient way. But this is
> > > > not a
> > > > >> > big
> > > > >> > > > > > concern
> > > > >> > > > > > > > for
> > > > >> > > > > > > > > the
> > > > >> > > > > > > > > > > > > > default
> > > > >> > > > > > > > > > > > > > > > > topic
> > > > >> > > > > > > > > > > > > > > > > > > based RLMM as mentioned in the
> > > KIP.
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > ~Satish.
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > On Wed, 13 Jul 2022 at 18:48,
> > > Divij
> > > > >> > Vaidya
> > > > >> > > <
> > > > >> > > > > > > > > > > > > > > divijvaidy...@gmail.com>
> > > > >> > > > > > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > Thank you for your review
> > Luke.
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > Reg: is that would the new
> > > > >> > > > > > `RemoteLogSizeBytes`
> > > > >> > > > > > > > > metric
> > > > >> > > > > > > > > > > > be a
> > > > >> > > > > > > > > > > > > > > > > > performance
> > > > >> > > > > > > > > > > > > > > > > > > > overhead? Although we move the
> > > > >> > > calculation
> > > > >> > > > > to a
> > > > >> > > > > > > > > seperate
> > > > >> > > > > > > > > > > > API,
> > > > >> > > > > > > > > > > > > > we
> > > > >> > > > > > > > > > > > > > > > > still
> > > > >> > > > > > > > > > > > > > > > > > > > can't assume users will
> > > implement
> > > > a
> > > > >> > > > > > light-weight
> > > > >> > > > > > > > > method,
> > > > >> > > > > > > > > > > > > right?
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > This metric would be logged
> > > using
> > > > >> the
> > > > >> > > > > > information
> > > > >> > > > > > > > > that is
> > > > >> > > > > > > > > > > > > > already
> > > > >> > > > > > > > > > > > > > > > > being
> > > > >> > > > > > > > > > > > > > > > > > > > calculated for handling remote
> > > > >> > retention
> > > > >> > > > > logic,
> > > > >> > > > > > > > > hence, no
> > > > >> > > > > > > > > > > > > > > > additional
> > > > >> > > > > > > > > > > > > > > > > > work
> > > > >> > > > > > > > > > > > > > > > > > > > is required to calculate this
> > > > >> metric.
> > > > >> > > More
> > > > >> > > > > > > > > specifically,
> > > > >> > > > > > > > > > > > > > whenever
> > > > >> > > > > > > > > > > > > > > > > > > > RemoteLogManager calls
> > > > >> getRemoteLogSize
> > > > >> > > > API,
> > > > >> > > > > > this
> > > > >> > > > > > > > > metric
> > > > >> > > > > > > > > > > > > would
> > > > >> > > > > > > > > > > > > > be
> > > > >> > > > > > > > > > > > > > > > > > > captured.
> > > > >> > > > > > > > > > > > > > > > > > > > This API call is made every
> > time
> > > > >> > > > > > RemoteLogManager
> > > > >> > > > > > > > > wants
> > > > >> > > > > > > > > > > to
> > > > >> > > > > > > > > > > > > > handle
> > > > >> > > > > > > > > > > > > > > > > > expired
> > > > >> > > > > > > > > > > > > > > > > > > > remote log segments (which
> > > should
> > > > be
> > > > >> > > > > periodic).
> > > > >> > > > > > > > Does
> > > > >> > > > > > > > > that
> > > > >> > > > > > > > > > > > > > address
> > > > >> > > > > > > > > > > > > > > > > your
> > > > >> > > > > > > > > > > > > > > > > > > > concern?
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > On Tue, Jul 12, 2022 at 11:01
> > AM
> > > > >> Luke
> > > > >> > > Chen
> > > > >> > > > <
> > > > >> > > > > > > > > > > > > show...@gmail.com>
> > > > >> > > > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > Hi Divij,
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > Thanks for the KIP!
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > I think it makes sense to
> > > > delegate
> > > > >> > the
> > > > >> > > > > > > > > responsibility
> > > > >> > > > > > > > > > > of
> > > > >> > > > > > > > > > > > > > > > > calculation
> > > > >> > > > > > > > > > > > > > > > > > to
> > > > >> > > > > > > > > > > > > > > > > > > > the
> > > > >> > > > > > > > > > > > > > > > > > > > > specific
> > > > RemoteLogMetadataManager
> > > > >> > > > > > > implementation.
> > > > >> > > > > > > > > > > > > > > > > > > > > But one thing I'm not quite
> > > > sure,
> > > > >> is
> > > > >> > > that
> > > > >> > > > > > would
> > > > >> > > > > > > > > the new
> > > > >> > > > > > > > > > > > > > > > > > > > > `RemoteLogSizeBytes` metric
> > > be a
> > > > >> > > > > performance
> > > > >> > > > > > > > > overhead?
> > > > >> > > > > > > > > > > > > > > > > > > > > Although we move the
> > > calculation
> > > > >> to a
> > > > >> > > > > > seperate
> > > > >> > > > > > > > > API, we
> > > > >> > > > > > > > > > > > > still
> > > > >> > > > > > > > > > > > > > > > can't
> > > > >> > > > > > > > > > > > > > > > > > > assume
> > > > >> > > > > > > > > > > > > > > > > > > > > users will implement a
> > > > >> light-weight
> > > > >> > > > method,
> > > > >> > > > > > > > right?
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > Thank you.
> > > > >> > > > > > > > > > > > > > > > > > > > > Luke
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > On Fri, Jul 1, 2022 at 5:47
> > PM
> > > > >> Divij
> > > > >> > > > > Vaidya <
> > > > >> > > > > > > > > > > > > > > > > divijvaidy...@gmail.com
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-852%3A+Optimize+calculation+of+size+for+log+in+remote+tier
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > > Hey folks
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > > Please take a look at this
> > > KIP
> > > > >> > which
> > > > >> > > > > > proposes
> > > > >> > > > > > > > an
> > > > >> > > > > > > > > > > > > extension
> > > > >> > > > > > > > > > > > > > to
> > > > >> > > > > > > > > > > > > > > > > > > KIP-405.
> > > > >> > > > > > > > > > > > > > > > > > > > > This
> > > > >> > > > > > > > > > > > > > > > > > > > > > is my first KIP with
> > Apache
> > > > >> Kafka
> > > > >> > > > > community
> > > > >> > > > > > > so
> > > > >> > > > > > > > > any
> > > > >> > > > > > > > > > > > > feedback
> > > > >> > > > > > > > > > > > > > > > would
> > > > >> > > > > > > > > > > > > > > > > > be
> > > > >> > > > > > > > > > > > > > > > > > > > > highly
> > > > >> > > > > > > > > > > > > > > > > > > > > > appreciated.
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > > Cheers!
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > > > --
> > > > >> > > > > > > > > > > > > > > > > > > > > > Divij Vaidya
> > > > >> > > > > > > > > > > > > > > > > > > > > > Sr. Software Engineer
> > > > >> > > > > > > > > > > > > > > > > > > > > > Amazon
> > > > >> > > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > > >
> > > > >> > > > > > > > > > > > >
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > >
> > >
> >

Reply via email to