Yes Luke, I am also in favour of creating producer snapshot at run time if foundEmpty as this would only be required for topics migrated from < 2.8 version. This will not break the existing contract with the plugin. Yes, metrics do not make sense here as of now. Greg, @Kamal Chandraprakash <kamal.chandraprak...@gmail.com> WDYT ? Arpit Goyal 8861094754
On Sat, Mar 23, 2024 at 3:05 PM Luke Chen <show...@gmail.com> wrote: > Hi Arpit, > > I'm in favor of creating an empty producer snapshot since it's only for > topics <= v2.8. > About the metric, I don't know what we expect users to know. > I think we can implement with the empty producer snapshot method, without > the metric. > And add them if users are requested it. > WDYT? > > Thank you. > Luke > > On Sat, Mar 23, 2024 at 1:24 PM Arpit Goyal <goyal.arpit...@gmail.com> > wrote: > > > Hi Team, > > Any further comments or suggestions on the possible approaches discussed > > above. > > > > On Tue, Mar 19, 2024, 09:55 Arpit Goyal <goyal.arpit...@gmail.com> > wrote: > > > > > @Luke Chen @Kamal Chandraprakash <kamal.chandraprak...@gmail.com> > @Greg > > > Harris Any suggestion on the above two possible approaches. > > > On Sun, Mar 17, 2024, 13:36 Arpit Goyal <goyal.arpit...@gmail.com> > > wrote: > > > > > >> > > >>>> In summary , There are two possible solution to handle the above > > >> scenario when producer snapshot file found to be null > > >> > > >> 1. *Generate empty producer snapshot file at run time when copying > > >> LogSegment* > > >> > > >> > > >> - This will not require any backward compatibility dependencies > with > > >> the plugin. > > >> - It preserves the contract i.e producerSnapshot files should be > > >> mandatory. > > >> - We could have a metric which helps us to assess how many times > > >> empty snapshot files have been created. > > >> > > >> 2. * Make producerSnapshot files optional * > > >> > > >> - This would break the contract with the plugin and would require > > >> defining a set of approaches to handle it which is mentioned > earlier > > in the > > >> thread. > > >> - If we make producer Snapshot optional , We would not be > handling > > >> the error which @LukeChen mentioned when producerSnapshot > > >> accidentally deleted a given segment. But this holds true for > > >> TransactionalIndex. > > >> - The other question is do we really need to make the field > optional. > > >> The only case where this problem can occur is only when the topic > > migrated > > >> from < 2.8 version. > > >> > > >> > > >
