Looks like this KIP has gone quiet for a bit, but here with an update (maybe this will revitalize the conversation too :) )
For KIP-1022 we want to introduce more features and will have a config ` unstable.version.enable` I know that this KIP proposed a config `unstable.metadata.version.enable` so just want to figure out how these will interact. I would propose just replacing the metadata specific one, but I know this config is actually already in code and being used, so this makes it a bit more tricky. An alternative approach is to have both configs. The metadata config would just control metadata, and the general one all features. What do folks think, Justine On Mon, Jan 29, 2024 at 1:42 PM Jun Rao <j...@confluent.io.invalid> wrote: > Hi, Colin, > > Thanks for the reply. > > "If Y changed the same file as X, we can just edit the file so that Y's > change come first." > > It seems the effort is more than just editing the file. One has to change > all the logic around the old IV. Also, what happens to the client? A client > may have implemented a version for a request corresponding to an unstable > feature. When the unstable feature is re-ordered, do all clients (including > those non-java ones) need to change the PRC implementation for the existing > underlying requests? > > Jun > > On Mon, Jan 29, 2024 at 9:57 AM Colin McCabe <cmcc...@apache.org> wrote: > > > On Tue, Jan 23, 2024, at 11:21, Jun Rao wrote: > > > Hi, Proven, > > > > > > Thanks for the KIP. > > > > > > I am not sure about the reordering approach proposed in the KIP. Let's > > say > > > in a release we have features X and Y, depending on MV IV1 and IV2, > > > respectively. At the release time, feature Y is ready, but X is not. I > > > guess the proposal is to move IV1 to a new MV IV3? > > > > Hi Jun, > > > > In your example, if X is not ready, it should be moved into an unstable > > MV. Then if Y is ready, it can be moved into a stable MV and we can > advance > > the last stable MV. > > > > > The issue is that IV2 > > > could have made changes on top of IV1. For example, IV2 could have > > evolved > > > the schema of the same inter broker request as IV1. In that case, what > > does > > > IV3 represent? We can't simply take the changes associated with IV1 > since > > > it could have conflicts with IV2. > > > > If Y changed the same file as X, we can just edit the file so that Y's > > change come first. > > > > Nobody using a stable MV should be affected, since they will be > generating > > records on an older (stable) version. > > > > > Even when there are no conflicts, I am not sure if the approach > supports > > > the trunk deployment model. Let's say we move two unstable MV IV3 and > IV4 > > > to IV7 and IV8. If someone deployed code corresponding to IV4 and later > > > upgraded the code to IV7, he/she (1) wouldn't be able to set IV4 in the > > > upgrade code since it no longer exists and (2) would be surprised that > > the > > > request protocol that worked under IV4 doesn't work now. > > > > > > > Upgrades are not supported if you are running an unstable MV. It's > > intended only for testing. > > > > Colin > > > > > > > > Thanks, > > > > > > Jun > > > > > > On Fri, Jan 19, 2024 at 2:13 PM Artem Livshits > > > <alivsh...@confluent.io.invalid> wrote: > > > > > >> Hi Colin, > > >> > > >> > I think feature flags are somewhat orthogonal to the stable / > > unstable > > >> discussion > > >> > > >> I think feature flags can be used as an alternative to achieve similar > > >> results as stable / unstable functionality. As well as long-lived > > feature > > >> branches. In my experience, I've seen feature flags to be more > > successful > > >> than feature branches for changes of existing functionality. I also > > think > > >> that stable / unstable MV would work better than feature branches. I > > just > > >> wanted to mention it for completeness, not sure if we should start a > > full > > >> fledged discussion on these topics. > > >> > > >> > I'm struggling a bit with your phrasing. Are you suggesting that > > unstable > > >> MVs would not be able to be in trunk? > > >> > > >> Unstable MV should be in trunk. The wording is related to when we can > > >> promote "unstable" to "stable". > > >> > > >> -Artem > > >> > > >> > > >> On Mon, Jan 15, 2024 at 10:03 PM Colin McCabe <cmcc...@apache.org> > > wrote: > > >> > > >> > On Fri, Jan 12, 2024, at 11:32, Artem Livshits wrote: > > >> > > I think using feature flags (whether we support a framework and > > tooling > > >> > for > > >> > > feature flags or just an ad-hoc XyzEnabled flag) can be an > > alternative > > >> to > > >> > > this KIP. I think the value of this KIP is that it's trying to > > >> propose a > > >> > > systemic approach for gating functionality that may take multiple > > >> > releases > > >> > > to develop. A problem with ad-hoc feature flags is that it's > useful > > >> > during > > >> > > feature development, so that people who are working on this > feature > > (or > > >> > are > > >> > > interested in beta-testing the feature) can get early access > > (without > > >> any > > >> > > guarantees on compatibility or even correctness); but then the > > feature > > >> > > flags often stick forever after the feature development is done > > (and as > > >> > > time moves one, the new code is written in such a way that it's > not > > >> > > possible to turn the feature off any more cleanly). > > >> > > > > >> > > > >> > Hi Artem, > > >> > > > >> > I think feature flags are somewhat orthogonal to the stable / > unstable > > >> > discussion. Even if every new feature was a feature flag, you > probably > > >> > still wouldn't want to stabilize the features immediately, to avoid > > >> > maintaining a lot of alpha stuff forever. > > >> > > > >> > (I also think that feature flags should be used sparingly, if at > all, > > >> > because of the way that they exponentially increase the test matrix. > > But > > >> > that's a tangent, I think, given the first point...) > > >> > > > >> > > > > >> > > I'd also clarify how I think about "stable". Ismael made a > comment > > " > > >> > > something is stable in the "this is battle-tested" sense.". I > don't > > >> > think > > >> > > it has to be "battle-tested", it just has to meet the bar of being > > in > > >> the > > >> > > trunk. Again, thinking of a small single-commit feature -- to > > commit > > >> to > > >> > > trunk, the feature doesn't have to be "battle-tested", but it > > should be > > >> > > complete (and not just a bunch of TODOs), with tests written and > > some > > >> > level > > >> > > of dev-testing done, so that once the release is cut, we can find > > and > > >> fix > > >> > > bugs and make it release-quality (as opposed to reverting the > whole > > >> > > thing). We can apply the same "stability" bar for features to be > in > > >> the > > >> > > stable MV -- fully complete, tests written and some level of dev > > >> testing > > >> > > done. > > >> > > > > >> > > > >> > I'm struggling a bit with your phrasing. Are you suggesting that > > unstable > > >> > MVs would not be able to be in trunk? I think we do want them to be > > able > > >> to > > >> > go into trunk. If they have to go into a branch, then there is > > actually > > >> no > > >> > need for any of this. > > >> > > > >> > Doing big features in long-lived branches is one genuine alternative > > to > > >> > unstable MVs, I think. But it's not an alternative that works well > > with > > >> our > > >> > current tooling for continuous integration, deployment, building, > > etc. I > > >> > think it would also impact developer productivity somewhat > negatively. > > >> > > > >> > best, > > >> > Colin > > >> > > > >> > > > >> > > > > >> > > -Artem > > >> > > > > >> > > On Fri, Jan 12, 2024 at 10:12 AM Justine Olshan > > >> > > <jols...@confluent.io.invalid> wrote: > > >> > > > > >> > >> Hi Ismael, > > >> > >> > > >> > >> I debated including something about feature flags in my last > > comment, > > >> > but > > >> > >> maybe I should have. > > >> > >> What you said makes sense. > > >> > >> > > >> > >> Justine > > >> > >> > > >> > >> On Fri, Jan 12, 2024 at 9:31 AM Ismael Juma <m...@ismaeljuma.com> > > >> wrote: > > >> > >> > > >> > >> > Justine, > > >> > >> > > > >> > >> > For features that are not production-ready, they should have an > > >> > >> additional > > >> > >> > configuration (not the metadata version) that enables/disables > > it. > > >> > The MV > > >> > >> > specific features we ship are something we have to support and > > make > > >> > sure > > >> > >> we > > >> > >> > don't break going forward. > > >> > >> > > > >> > >> > Ismael > > >> > >> > > > >> > >> > On Fri, Jan 12, 2024 at 9:26 AM Justine Olshan > > >> > >> > <jols...@confluent.io.invalid> > > >> > >> > wrote: > > >> > >> > > > >> > >> > > Hi Ismael, > > >> > >> > > > > >> > >> > > I think the concern I have about a MV for a feature that is > not > > >> > >> > production > > >> > >> > > ready is that it blocks any development/features with higher > MV > > >> > >> versions > > >> > >> > > that could be production ready. > > >> > >> > > > > >> > >> > > I do see your point though. Previously MV/IBP was about pure > > >> broker > > >> > >> > > compatibility and not about the confidence in the feature it > is > > >> > >> gating. I > > >> > >> > > do wonder though if it could be useful to have that sort of > > >> gating. > > >> > >> > > I also wonder if an internal config could be useful so that > the > > >> > average > > >> > >> > > user doesn't have to worry about the complexities of unstable > > >> > metadata > > >> > >> > > versions (and their risks). > > >> > >> > > > > >> > >> > > I am ok with options 2 and 2 as well by the way. > > >> > >> > > > > >> > >> > > Justine > > >> > >> > > > > >> > >> > > On Fri, Jan 12, 2024 at 7:36 AM Ismael Juma < > m...@ismaeljuma.com > > > > > >> > wrote: > > >> > >> > > > > >> > >> > > > Hi, > > >> > >> > > > > > >> > >> > > > Thanks for the KIP. > > >> > >> > > > > > >> > >> > > > Reading the discussion, I think a lot of the confusion is > > due to > > >> > the > > >> > >> > > > "unstable" naming. People are then trying to figure out > when > > we > > >> > think > > >> > >> > > > something is stable in the "this is battle-tested" sense. > But > > >> this > > >> > >> flag > > >> > >> > > > should not be about that. We can have an MV for a feature > > that > > >> is > > >> > not > > >> > >> > yet > > >> > >> > > > production-ready (and we did that when KRaft itself was not > > >> > >> production > > >> > >> > > > ready). I think this flag is about metadata versions that > are > > >> > >> basically > > >> > >> > > > unsupported - if you use it, you get to keep the pieces. > They > > >> > exist > > >> > >> > > solely > > >> > >> > > > to make the lives of Apache Kafka developers easier. I > would > > >> even > > >> > >> > suggest > > >> > >> > > > that the config we introduce be of the internal variety, ie > > it > > >> > won't > > >> > >> > show > > >> > >> > > > in the generated documentation and there won't be any > > >> > compatibility > > >> > >> > > > guarantee. > > >> > >> > > > > > >> > >> > > > Thoughts? > > >> > >> > > > > > >> > >> > > > Ismael > > >> > >> > > > > > >> > >> > > > On Fri, Jan 5, 2024 at 7:33 AM Proven Provenzano > > >> > >> > > > <pprovenz...@confluent.io.invalid> wrote: > > >> > >> > > > > > >> > >> > > > > Hey folks, > > >> > >> > > > > > > >> > >> > > > > I am starting a discussion thread for managing unstable > > >> metadata > > >> > >> > > versions > > >> > >> > > > > in Apache Kafka. > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1014%3A+Managing+Unstable+Metadata+Versions+in+Apache+Kafka > > >> > >> > > > > > > >> > >> > > > > This KIP is actually already implemented in 3.7 with PR > > >> > >> > > > > https://github.com/apache/kafka/pull/14860. > > >> > >> > > > > I have created this KIP to explain the motivation and how > > >> > managing > > >> > >> > > > Metadata > > >> > >> > > > > Versions is expected to work. > > >> > >> > > > > Comments are greatly appreciated as this process can > > always be > > >> > >> > > improved. > > >> > >> > > > > > > >> > >> > > > > -- > > >> > >> > > > > --Proven > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > > > >> > > >
