Re: [DISCUSS] Adding dependency on agrona

2022-10-01 Thread Benedict
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  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  wrote:
>> Also +1
>> 
 On 23 Sep 2022, at 18:17, David Capwell  wrote:
 
>>> +1 from me
>>> 
 On Sep 23, 2022, at 1:21 AM, Branimir Lambov 
  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  
> 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  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  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 and   |
> | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org |
> | 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 availab

Re: [DISCUSS] Adding dependency on agrona

2022-10-01 Thread Derek Chen-Becker
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  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  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  wrote:
>
>> Also +1
>>
>> On 23 Sep 2022, at 18:17, David Capwell  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 
>> 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  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  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
>>> 
>>> and   |
>>> | https://pgp.mit.edu/pks/lookup?search

Re: [DISCUSS] Adding dependency on agrona

2022-10-01 Thread Dinesh Joshi
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  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  > 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 > > 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 > > wrote:
>> Also +1
>> 
>>> On 23 Sep 2022, at 18:17, David Capwell >> > wrote:
>>> 
>>> +1 from me
>>> 
 On Sep 23, 2022, at 1:21 AM, Branimir Lambov >>> > 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 >>> > 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 >>> > 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  > wrote:
> 
> 
> Hi everyone,
> 
> CASSANDRA-17240 (Trie memtable