Hi Arpit, I think creating empty producer snapshots would be backwards-compatible for the tiered storage plugins, but I'm not aware of what the other compatibility/design concerns might be. Maybe you or another reviewer can answer these questions: 1. Does an empty producer snapshot have the same behavior as a non-existent snapshot when restored? 2. Why were empty producer snapshots not backfilled for older data when clusters were upgraded from 2.8? 3. Do producer snapshots need to be available contiguously, or can earlier snapshots be empty while later segments do not exist?
Thanks, Greg On Sat, Mar 23, 2024 at 3:24 AM Arpit Goyal <[email protected]> wrote: > > 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 <[email protected]> WDYT ? > Arpit Goyal > 8861094754 > > > On Sat, Mar 23, 2024 at 3:05 PM Luke Chen <[email protected]> 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 <[email protected]> > > wrote: > > > > > Hi Team, > > > Any further comments or suggestions on the possible approaches discussed > > > above. > > > > > > On Tue, Mar 19, 2024, 09:55 Arpit Goyal <[email protected]> > > wrote: > > > > > > > @Luke Chen @Kamal Chandraprakash <[email protected]> > > @Greg > > > > Harris Any suggestion on the above two possible approaches. > > > > On Sun, Mar 17, 2024, 13:36 Arpit Goyal <[email protected]> > > > 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. > > > >> > > > >> > > > > >
