Jorge,
About API name: Good point. I have changed it to remoteLogSize instead of
getRemoteLogSize

About partition tag in the metric: We don't use partition tag across any of
the RemoteStorage metrics and I would like to keep this metric aligned with
the rest. I will change the metric though to type=BrokerTopicMetrics
instead of type=RemoteLogManager, since this is topic level information and
not specific to RemoteLogManager.


Satish,
Ah yes! Updated from "This would increase the broker start-up time." to
"This would increase the bootstrap time for the remote storage thread pool
before the first eligible segment is archived."

--
Divij Vaidya



On Mon, Jul 3, 2023 at 2:07 PM Satish Duggana <satish.dugg...@gmail.com>
wrote:

> Thanks Divij for taking the feedback and updating the motivation
> section in the KIP.
>
> One more comment on Alternative solution-3, The con is not valid as
> that will not affect the broker restart times as discussed in the
> earlier email in this thread. You may want to update that.
>
> ~Satish.
>
> On Sun, 2 Jul 2023 at 01:03, Divij Vaidya <divijvaidy...@gmail.com> wrote:
> >
> > Thank you folks for reviewing this KIP.
> >
> > Satish, I have modified the motivation to make it more clear. Now it
> says,
> > "Since the main feature of tiered storage is storing a large amount of
> > data, we expect num_remote_segments to be large. A frequent linear scan
> > (i.e. listing all segment metadata) could be expensive/slower because of
> > the underlying storage used by RemoteLogMetadataManager. This slowness to
> > list all segment metadata could result in the loss of availability...."
> >
> > Jun, Kamal, Satish, if you don't have any further concerns, I would
> > appreciate a vote for this KIP in the voting thread -
> > https://lists.apache.org/thread/soz00990gvzodv7oyqj4ysvktrqy6xfk
> >
> > --
> > Divij Vaidya
> >
> >
> >
> > On Sat, Jul 1, 2023 at 6:16 AM Kamal Chandraprakash <
> > kamal.chandraprak...@gmail.com> wrote:
> >
> > > Hi Divij,
> > >
> > > Thanks for the explanation. LGTM.
> > >
> > > --
> > > Kamal
> > >
> > > On Sat, Jul 1, 2023 at 7:28 AM Satish Duggana <
> satish.dugg...@gmail.com>
> > > wrote:
> > >
> > > > 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