> Providing some guidance seems like a reasonable step to take. Would that be > in our coding guidelines To my knowledge we don't currently codify guidance on idiomatic usage of various things in the project, but if we did, that'd probably be the place to put them.
Also, fwiw, I think we should. The more we can help bring new contributors up to speed and improve the consistency of the codebase in general the better. On Sat, Oct 1, 2022, at 3:35 PM, Dinesh Joshi wrote: > I am in favor of adding the dependency but like many others have already > said, it would be nice to outline what and why are we using it? > > Dinesh > >> On Oct 1, 2022, at 9:57 AM, Derek Chen-Becker <de...@chen-becker.org> wrote: >> >> Providing some guidance seems like a reasonable step to take. Would that be >> in our coding guidelines >> (https://cassandra.apache.org/_/development/code_style.html) or do we have >> other places where we provide development guidelines? I like your optimism >> on Valhalla's timeline, but I also agree that there's no need to change >> things that aren't currently problems :) >> >> Cheers, >> >> Derek >> >> On Sat, Oct 1, 2022 at 6:51 AM Benedict <bened...@apache.org> wrote: >>> >>> I agree we need to be cautious about permitting overlapping functionality. >>> I think if there are other utilities that duplicate existing functionality >>> we should have a similar discussion on list before we begin using it, even >>> after including the library. Perhaps even our wording around seeking input >>> should be modified to include functionality present in a dependency but >>> unused, and that may duplicate existing functionality. >>> >>> I am unconvinced it would be worthwhile replacing the use of ByteBuffer >>> more generally within the codebase though, as Project Valhalla will likely >>> cause us to revisit our internal APIs significantly - and that may arrive >>> in some form within the next couple of years. So the value of the churn of >>> changing implementation is unlikely to be high. >>> >>> That said, isolated subsystems could probably use it if we think it’s >>> superior for the time being. >>> >>> Just my 2c. >>> >>>> On 29 Sep 2022, at 15:26, Derek Chen-Becker <de...@chen-becker.org> wrote: >>>> >>>> +1 from me with some concerns. >>>> >>>> Agrona looks like a nice lib, and I like what I've seen skimming the docs, >>>> but I think there's a longer-term discussion around whether we want to do >>>> anything about bringing in a library that effectively duplicates a bunch >>>> of standard lib constructs. My main concern is cognitive load if there are >>>> subtle semantic differences, say, between Agrona and stdlib buffers or any >>>> other construct where we might have both in the codebase. If we think >>>> Agrona has better buffers (or locking, timers, etc), should we be talking >>>> about replacing usage of stdlib with Agrona throughout, or making a >>>> recommendation for one over the other for future work? >>>> >>>> Cheers, >>>> >>>> Derek >>>> >>>> On Thu, Sep 29, 2022 at 12:26 AM Benedict <bened...@apache.org> wrote: >>>>> >>>>> Also +1 >>>>> >>>>> >>>>>> On 23 Sep 2022, at 18:17, David Capwell <dcapw...@apple.com> wrote: >>>>>> +1 from me >>>>>> >>>>>>> On Sep 23, 2022, at 1:21 AM, Branimir Lambov >>>>>>> <branimir.lam...@datastax.com> wrote: >>>>>>> >>>>>>> The usage in the trie memtable is only for volatile access to buffers. >>>>>>> In this case I chose the library instead of reimplementing the >>>>>>> functionality (e.g. as methods in `ByteBufferUtil`) because the >>>>>>> relevant interface makes sense and the library is a good quality one >>>>>>> that contains a range of other utilities that can be very useful to >>>>>>> Cassandra. >>>>>>> >>>>>>> In other words, I personally would welcome opening Cassandra up to >>>>>>> using other parts of Agrona, and am asking if the community shares this >>>>>>> sentiment. >>>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> Branimir >>>>>>> >>>>>>> On Wed, Sep 21, 2022 at 9:15 PM Derek Chen-Becker >>>>>>> <de...@chen-becker.org> wrote: >>>>>>>> Agrona looks like it has quite a bit more than just buffers, so if we >>>>>>>> add this as a dependency for the new memtable, would it potentially >>>>>>>> open up use of other parts of Agrona (wittingly or not)? Unless I >>>>>>>> misunderstood, wasn't part of the new memtable implementation an >>>>>>>> interface to allow this to be pluggable? Could we avoid bringing it in >>>>>>>> as a full dependency for Cassandra if the trie memtable were packaged >>>>>>>> separately as a plugin instead of being included directly? >>>>>>>> >>>>>>>> Cheers, >>>>>>>> >>>>>>>> Derek >>>>>>>> >>>>>>>> On Wed, Sep 21, 2022 at 6:41 AM Benedict <bened...@apache.org> wrote: >>>>>>>>> >>>>>>>>> In principle no, it’s a high quality library. But it might help to >>>>>>>>> briefly outline what it’s used for. I assume it is instead of >>>>>>>>> ByteBuffer? In which case it could maybe be worthwhile discussing as >>>>>>>>> a project how we foresee interaction with existing buffer machinery, >>>>>>>>> and maybe how we expect our buffer use to evolve on the project, as >>>>>>>>> we already have several buffers. >>>>>>>>> >>>>>>>>> That said, I anticipate our buffer use changing significantly with >>>>>>>>> the introduction of value types and native memory improvements coming >>>>>>>>> in future Java releases, so my personal inclination is just to accept >>>>>>>>> the dependency. >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 21 Sep 2022, at 13:29, Branimir Lambov <blam...@apache.org> wrote: >>>>>>>>>> >>>>>>>>>> Hi everyone, >>>>>>>>>> >>>>>>>>>> CASSANDRA-17240 (Trie memtable implementation) introduces a >>>>>>>>>> dependency on the agrona library >>>>>>>>>> (https://github.com/real-logic/agrona). >>>>>>>>>> >>>>>>>>>> Does anyone have any objections to adding this dependency? >>>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> Branimir >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> +---------------------------------------------------------------+ >>>>>>>> | Derek Chen-Becker | >>>>>>>> | GPG Key available at https://keybase.io/dchenbecker >>>>>>>> <https://urldefense.com/v3/__https://keybase.io/dchenbecker__;!!PbtH5S7Ebw!cY9TyIm1RqAGMkhgyKDjzQcOq6Cy6kzMj_VjvMm40JG9VMm6JgFfH9omG1Spx0UmlkEcGJcFmDtKjcbIGBN7PBunbg$> >>>>>>>> and | >>>>>>>> | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org >>>>>>>> <https://urldefense.com/v3/__https://pgp.mit.edu/pks/lookup?search=derek*40chen-becker.org__;JQ!!PbtH5S7Ebw!cY9TyIm1RqAGMkhgyKDjzQcOq6Cy6kzMj_VjvMm40JG9VMm6JgFfH9omG1Spx0UmlkEcGJcFmDtKjcbIGBPzYoayyA$> >>>>>>>> | >>>>>>>> | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7 7F42 AFC5 AFEE 96E4 6ACC | >>>>>>>> +---------------------------------------------------------------+ >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Branimir Lambov >>>>>>> e. branimir.lam...@datastax.com >>>>>>> w. www.datastax.com >>>>>>> >>>>>> >>>> >>>> >>>> -- >>>> +---------------------------------------------------------------+ >>>> | Derek Chen-Becker | >>>> | GPG Key available at https://keybase.io/dchenbecker and | >>>> | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org | >>>> | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7 7F42 AFC5 AFEE 96E4 6ACC | >>>> +---------------------------------------------------------------+ >>>> >> >> >> -- >> +---------------------------------------------------------------+ >> | Derek Chen-Becker | >> | GPG Key available at https://keybase.io/dchenbecker and | >> | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org | >> | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7 7F42 AFC5 AFEE 96E4 6ACC | >> +---------------------------------------------------------------+ >>
