Hi Colin, Thanks for the response. I have no more comments. +1 (binding)
Luke On Thu, Jun 8, 2023 at 6:02 AM Colin McCabe <cmcc...@apache.org> wrote: > > Hi Luke, > > Thanks for the review and the suggestion. > > I think we will add more "handling time" metrics later, but for now I don't > want to make this KIP any bigger than it is already... > > best, > Colin > > > On Wed, Jun 7, 2023, at 03:12, Luke Chen wrote: > > Hi Colin, > > > > One comment: > > Should we add a metric to record the snapshot handling time? > > Since we know the snapshot loading might take long if the size is huge. > > We might want to know how much time it is processed. WDYT? > > > > No matter you think we need it or not, the KIP LGTM. > > +1 from me. > > > > > > Thank you. > > Luke > > > > On Wed, Jun 7, 2023 at 1:33 PM Colin McCabe <cmcc...@apache.org> wrote: > >> > >> Hi all, > >> > >> I added two new metrics to the list: > >> > >> * LatestSnapshotGeneratedBytes > >> * LatestSnapshotGeneratedAgeMs > >> > >> These will help monitor the period snapshot generation process. > >> > >> best, > >> Colin > >> > >> > >> On Tue, Jun 6, 2023, at 22:21, Colin McCabe wrote: > >> > Hi Divij, > >> > > >> > Yes, I am referring to the feature level. I changed the description of > >> > CurrentMetadataVersion to reference the feature level specifically. > >> > > >> > best, > >> > Colin > >> > > >> > > >> > On Tue, Jun 6, 2023, at 05:56, Divij Vaidya wrote: > >> >> "Each metadata version has a corresponding integer in the > >> >> MetadataVersion.java file." > >> >> > >> >> Please correct me if I'm wrong, but are you referring to "featureLevel" > >> >> in > >> >> the enum at > >> >> https://github.com/apache/kafka/blob/trunk/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java#L45 > >> >> ? Is yes, can we please update the description of the metric to make it > >> >> easier for the users to understand this? For example, we can say, > >> >> "Represents the current metadata version as an integer value. See > >> >> MetadataVersion (hyperlink) for a mapping between string and integer > >> >> formats of metadata version". > >> >> > >> >> -- > >> >> Divij Vaidya > >> >> > >> >> > >> >> > >> >> On Tue, Jun 6, 2023 at 1:51 PM Ron Dagostino <rndg...@gmail.com> wrote: > >> >> > >> >>> Thanks again for the KIP, Colin. +1 (binding). > >> >>> > >> >>> Ron > >> >>> > >> >>> > On Jun 6, 2023, at 7:02 AM, Igor Soarez <soa...@apple.com.invalid> > >> >>> wrote: > >> >>> > > >> >>> > Thanks for the KIP. > >> >>> > > >> >>> > Seems straightforward, LGTM. > >> >>> > Non binding +1. > >> >>> > > >> >>> > -- > >> >>> > Igor > >> >>> > > >> >>>
