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