Re: [DISCUSS] Change style guide to recommend use of @Override
Thanks all! Sent a patch out in https://issues.apache.org/jira/browse/CASSANDRA-16096 and tested the change autogenerates the annotation now (if using Intellij it is best to delete the project and regenerate it as the change didn't take without deleting in intellij). On Wed, Sep 2, 2020 at 10:22 AM Blake Eggleston wrote: > +1 > > > On Sep 1, 2020, at 11:27 AM, David Capwell wrote: > > > > Currently our style guide recommends to avoid using @Override and updates > > intellij's code style to exclude it by default; I would like to propose > we > > change this recommendation to use it and to update intellij's style to > > include it by default. > > > > @Override is used by javac to enforce that a method is in fact overriding > > from an abstract class or an interface and if this stops being true (such > > as a refactor happens) then a compiler error is thrown; when we default > to > > excluding, it makes it harder to detect that a refactor catches all > > implementations and can lead to subtle and hard to track down bugs. > > > > This proposal is for new code and would not be to go rewrite all code at > > once, but would recommend new code adopt this style, and to pull old code > > forward which is related to changes being made (similar to our stance on > > imports). > > > > If people are ok with this, I will file a JIRA, update the docs, and > > update intellij's formatting. > > > > Thanks for your time! > > > - > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > For additional commands, e-mail: dev-h...@cassandra.apache.org > >
Re: [DISCUSS] Change style guide to recommend use of @Override
+1 > On Sep 1, 2020, at 11:27 AM, David Capwell wrote: > > Currently our style guide recommends to avoid using @Override and updates > intellij's code style to exclude it by default; I would like to propose we > change this recommendation to use it and to update intellij's style to > include it by default. > > @Override is used by javac to enforce that a method is in fact overriding > from an abstract class or an interface and if this stops being true (such > as a refactor happens) then a compiler error is thrown; when we default to > excluding, it makes it harder to detect that a refactor catches all > implementations and can lead to subtle and hard to track down bugs. > > This proposal is for new code and would not be to go rewrite all code at > once, but would recommend new code adopt this style, and to pull old code > forward which is related to changes being made (similar to our stance on > imports). > > If people are ok with this, I will file a JIRA, update the docs, and > update intellij's formatting. > > Thanks for your time! - To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
Re: [DISCUSS] Change style guide to recommend use of @Override
+1. Some of us have already adopted this change some time ago, but it’d be nice to make it official for everybody. > On 1 Sep 2020, at 19:27, David Capwell wrote: > > Currently our style guide recommends to avoid using @Override and updates > intellij's code style to exclude it by default; I would like to propose we > change this recommendation to use it and to update intellij's style to > include it by default. > > @Override is used by javac to enforce that a method is in fact overriding > from an abstract class or an interface and if this stops being true (such > as a refactor happens) then a compiler error is thrown; when we default to > excluding, it makes it harder to detect that a refactor catches all > implementations and can lead to subtle and hard to track down bugs. > > This proposal is for new code and would not be to go rewrite all code at > once, but would recommend new code adopt this style, and to pull old code > forward which is related to changes being made (similar to our stance on > imports). > > If people are ok with this, I will file a JIRA, update the docs, and > update intellij's formatting. > > Thanks for your time! - To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
Re: [DISCUSS] Change style guide to recommend use of @Override
+1 — Robert Stupp @snazy > On 2. Sep 2020, at 10:38, Sylvain Lebresne wrote: > > +1 > -- > Sylvain > > > On Wed, Sep 2, 2020 at 10:21 AM Sam Tunnicliffe wrote: > >> +1 >> >>> On 2 Sep 2020, at 09:03, Benjamin Lerer >> wrote: >>> >>> +1 >>> >>> >>> >>> On Wed, Sep 2, 2020 at 5:36 AM Berenguer Blasi >> >>> wrote: >>> +1 On 2/9/20 5:09, Joshua McKenzie wrote: > +1 > > On Tue, Sep 1, 2020 at 6:26 PM Jordan West wrote: > >> +1 >> >> On Tue, Sep 1, 2020 at 12:22 PM Benedict Elliott Smith < >> bened...@apache.org> >> wrote: >> >>> +1 >>> >>> >>> >>> On 01/09/2020, 20:09, "Caleb Rackliffe" >> wrote: >>> >>> >>> +1 >>> >>> >>> >>> On Tue, Sep 1, 2020, 2:00 PM Jasonstack Zhao Yang < >>> jasonstack.z...@gmail.com> >>> >>> wrote: >>> >>> >>> +1 >>> >>> On Wed, 2 Sep 2020 at 02:45, Dinesh Joshi >> wrote: >>> > +1 >>> > >>> >> On Sep 1, 2020, at 11:27 AM, David Capwell < dcapw...@gmail.com >>> >>> wrote: >>> >> >>> >> Currently our style guide recommends to avoid using @Override >> and updates >>> >> intellij's code style to exclude it by default; I would like to >>> propose >>> > we >>> >> change this recommendation to use it and to update intellij's >>> style to >>> >> include it by default. >>> >> >>> >> @Override is used by javac to enforce that a method is in fact >>> overriding >>> >> from an abstract class or an interface and if this stops being >>> true >>> (such >>> >> as a refactor happens) then a compiler error is thrown; when we >>> default >>> > to >>> >> excluding, it makes it harder to detect that a refactor catches >>> all >>> >> implementations and can lead to subtle and hard to track down >>> bugs. >>> >> >>> >> This proposal is for new code and would not be to go rewrite >> all >>> code >>> at >>> >> once, but would recommend new code adopt this style, and to >> pull >>> old >>> code >>> >> forward which is related to changes being made (similar to our >>> stance >>> on >>> >> imports). >>> >> >>> >> If people are ok with this, I will file a JIRA, update the >> docs, >>> and >>> >> update intellij's formatting. >>> >> >>> >> Thanks for your time! >>> > >>> > >>> > >>> - >>> > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >>> > For additional commands, e-mail: dev-h...@cassandra.apache.org >>> > >>> > >>> >>> >>> >>> >>> >>> >>> >>> >>> - >>> >>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >>> >>> For additional commands, e-mail: dev-h...@cassandra.apache.org >>> >>> >>> >>> - To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org >> >> >> - >> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >> For additional commands, e-mail: dev-h...@cassandra.apache.org >> >>
Re: [DISCUSS] Change style guide to recommend use of @Override
+1 -- Sylvain On Wed, Sep 2, 2020 at 10:21 AM Sam Tunnicliffe wrote: > +1 > > > On 2 Sep 2020, at 09:03, Benjamin Lerer > wrote: > > > > +1 > > > > > > > > On Wed, Sep 2, 2020 at 5:36 AM Berenguer Blasi > > > wrote: > > > >> +1 > >> > >> On 2/9/20 5:09, Joshua McKenzie wrote: > >>> +1 > >>> > >>> On Tue, Sep 1, 2020 at 6:26 PM Jordan West wrote: > >>> > +1 > > On Tue, Sep 1, 2020 at 12:22 PM Benedict Elliott Smith < > bened...@apache.org> > wrote: > > > +1 > > > > > > > > On 01/09/2020, 20:09, "Caleb Rackliffe" > wrote: > > > > > >+1 > > > > > > > >On Tue, Sep 1, 2020, 2:00 PM Jasonstack Zhao Yang < > > jasonstack.z...@gmail.com> > > > >wrote: > > > > > > > >> +1 > > > >> > > > >> On Wed, 2 Sep 2020 at 02:45, Dinesh Joshi > wrote: > >> > > > >>> +1 > > > >>> > > > On Sep 1, 2020, at 11:27 AM, David Capwell < > >> dcapw...@gmail.com > > > > wrote: > > > > > > Currently our style guide recommends to avoid using @Override > and > >> updates > > > intellij's code style to exclude it by default; I would like > >> to > > propose > > > >>> we > > > change this recommendation to use it and to update intellij's > > style to > > > include it by default. > > > > > > @Override is used by javac to enforce that a method is in > >> fact > > > >> overriding > > > from an abstract class or an interface and if this stops > >> being > > true > > > >> (such > > > as a refactor happens) then a compiler error is thrown; when > >> we > > default > > > >>> to > > > excluding, it makes it harder to detect that a refactor > >> catches > > all > > > implementations and can lead to subtle and hard to track down > > bugs. > > > > > > This proposal is for new code and would not be to go rewrite > all > > code > > > >> at > > > once, but would recommend new code adopt this style, and to > pull > > old > > > >> code > > > forward which is related to changes being made (similar to > >> our > > stance > > > >> on > > > imports). > > > > > > If people are ok with this, I will file a JIRA, update the > docs, > > and > > > update intellij's formatting. > > > > > > Thanks for your time! > > > >>> > > > >>> > > > >>> > > - > > > >>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > > > >>> For additional commands, e-mail: dev-h...@cassandra.apache.org > > > >>> > > > >>> > > > >> > > > > > > > > > > > > > > > > - > > > > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > > > > For additional commands, e-mail: dev-h...@cassandra.apache.org > > > > > > > > > >> > >> - > >> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > >> For additional commands, e-mail: dev-h...@cassandra.apache.org > >> > >> > > > - > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > For additional commands, e-mail: dev-h...@cassandra.apache.org > >
Re: [DISCUSS] Change style guide to recommend use of @Override
+1 > On 2 Sep 2020, at 09:03, Benjamin Lerer wrote: > > +1 > > > > On Wed, Sep 2, 2020 at 5:36 AM Berenguer Blasi > wrote: > >> +1 >> >> On 2/9/20 5:09, Joshua McKenzie wrote: >>> +1 >>> >>> On Tue, Sep 1, 2020 at 6:26 PM Jordan West wrote: >>> +1 On Tue, Sep 1, 2020 at 12:22 PM Benedict Elliott Smith < bened...@apache.org> wrote: > +1 > > > > On 01/09/2020, 20:09, "Caleb Rackliffe" wrote: > > >+1 > > > >On Tue, Sep 1, 2020, 2:00 PM Jasonstack Zhao Yang < > jasonstack.z...@gmail.com> > >wrote: > > > >> +1 > >> > >> On Wed, 2 Sep 2020 at 02:45, Dinesh Joshi wrote: >> > >>> +1 > >>> > On Sep 1, 2020, at 11:27 AM, David Capwell < >> dcapw...@gmail.com > > wrote: > > Currently our style guide recommends to avoid using @Override and >> updates > intellij's code style to exclude it by default; I would like >> to > propose > >>> we > change this recommendation to use it and to update intellij's > style to > include it by default. > > @Override is used by javac to enforce that a method is in >> fact > >> overriding > from an abstract class or an interface and if this stops >> being > true > >> (such > as a refactor happens) then a compiler error is thrown; when >> we > default > >>> to > excluding, it makes it harder to detect that a refactor >> catches > all > implementations and can lead to subtle and hard to track down > bugs. > > This proposal is for new code and would not be to go rewrite all > code > >> at > once, but would recommend new code adopt this style, and to pull > old > >> code > forward which is related to changes being made (similar to >> our > stance > >> on > imports). > > If people are ok with this, I will file a JIRA, update the docs, > and > update intellij's formatting. > > Thanks for your time! > >>> > >>> > >>> > - > >>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > >>> For additional commands, e-mail: dev-h...@cassandra.apache.org > >>> > >>> > >> > > > > > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > > For additional commands, e-mail: dev-h...@cassandra.apache.org > > > > >> >> - >> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >> For additional commands, e-mail: dev-h...@cassandra.apache.org >> >> - To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
Re: [DISCUSS] Change style guide to recommend use of @Override
+1 On Wed, Sep 2, 2020 at 5:36 AM Berenguer Blasi wrote: > +1 > > On 2/9/20 5:09, Joshua McKenzie wrote: > > +1 > > > > On Tue, Sep 1, 2020 at 6:26 PM Jordan West wrote: > > > >> +1 > >> > >> On Tue, Sep 1, 2020 at 12:22 PM Benedict Elliott Smith < > >> bened...@apache.org> > >> wrote: > >> > >>> +1 > >>> > >>> > >>> > >>> On 01/09/2020, 20:09, "Caleb Rackliffe" > >> wrote: > >>> > >>> > >>> +1 > >>> > >>> > >>> > >>> On Tue, Sep 1, 2020, 2:00 PM Jasonstack Zhao Yang < > >>> jasonstack.z...@gmail.com> > >>> > >>> wrote: > >>> > >>> > >>> > >>> > +1 > >>> > >>> > > >>> > >>> > On Wed, 2 Sep 2020 at 02:45, Dinesh Joshi > >> wrote: > >>> > > >>> > >>> > > +1 > >>> > >>> > > > >>> > >>> > > > On Sep 1, 2020, at 11:27 AM, David Capwell < > dcapw...@gmail.com > >>> > >>> wrote: > >>> > >>> > > > > >>> > >>> > > > Currently our style guide recommends to avoid using @Override > >> and > >>> > updates > >>> > >>> > > > intellij's code style to exclude it by default; I would like > to > >>> propose > >>> > >>> > > we > >>> > >>> > > > change this recommendation to use it and to update intellij's > >>> style to > >>> > >>> > > > include it by default. > >>> > >>> > > > > >>> > >>> > > > @Override is used by javac to enforce that a method is in > fact > >>> > >>> > overriding > >>> > >>> > > > from an abstract class or an interface and if this stops > being > >>> true > >>> > >>> > (such > >>> > >>> > > > as a refactor happens) then a compiler error is thrown; when > we > >>> default > >>> > >>> > > to > >>> > >>> > > > excluding, it makes it harder to detect that a refactor > catches > >>> all > >>> > >>> > > > implementations and can lead to subtle and hard to track down > >>> bugs. > >>> > >>> > > > > >>> > >>> > > > This proposal is for new code and would not be to go rewrite > >> all > >>> code > >>> > >>> > at > >>> > >>> > > > once, but would recommend new code adopt this style, and to > >> pull > >>> old > >>> > >>> > code > >>> > >>> > > > forward which is related to changes being made (similar to > our > >>> stance > >>> > >>> > on > >>> > >>> > > > imports). > >>> > >>> > > > > >>> > >>> > > > If people are ok with this, I will file a JIRA, update the > >> docs, > >>> and > >>> > >>> > > > update intellij's formatting. > >>> > >>> > > > > >>> > >>> > > > Thanks for your time! > >>> > >>> > > > >>> > >>> > > > >>> > >>> > > > >>> - > >>> > >>> > > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > >>> > >>> > > For additional commands, e-mail: dev-h...@cassandra.apache.org > >>> > >>> > > > >>> > >>> > > > >>> > >>> > > >>> > >>> > >>> > >>> > >>> > >>> > >>> > >>> - > >>> > >>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > >>> > >>> For additional commands, e-mail: dev-h...@cassandra.apache.org > >>> > >>> > >>> > >>> > > - > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > For additional commands, e-mail: dev-h...@cassandra.apache.org > >
Re: Check in on CASSANDRA-15393
Very much appreciated that this was raised here Blake, and the detailed write-up you provided. If 4.0-beta gets more such large improvement patches than it does testing and bug-fix patches, then the concern is we will never get to 4.0-rc. So I hope it is only obvious and valued that we do and continue to call them out during the beta phase. On Fri, 28 Aug 2020 at 02:33, Joshua McKenzie wrote: > No qualms with that here; it'll be a bit before GA so this has time to > soak. > > On Thu, Aug 27, 2020 at 8:21 PM Blake Eggleston > wrote: > > > Caleb is currently working through his second round of review, and Marcus > > said he's about halfway through his review as of this morning. So I'd > > expect it to be committed within a week or so. > > > > > On Aug 27, 2020, at 5:09 PM, Joshua McKenzie > > wrote: > > > > > > Is there an ETA on them landing? The later, the more risk to stability > of > > > GA due to lack of time soaking. > > > > > > On Thu, Aug 27, 2020 at 4:01 PM Blake Eggleston > > > wrote: > > > > > >> Hi dev@, > > >> > > >> Mick asked that I check in w/ the dev list about CASSANDRA-15393. > > There's > > >> some concern regarding the patch and it's suitability for inclusion in > > >> 4.0-beta. > > >> > > >> CASSANDRA-15393 reduces garbage created by compaction and the read > paths > > >> by about 25%. It's part of CASSANDRA-15387, which, including this > patch, > > >> reduces garbage from the read and compaction paths by about 50%. > > >> CASSANDRA-15393 does this by supporting byte array backed cell and > > >> clustering types, which is acheived by abstracting the backing type > > >> (ByteBuffer/byte[]) from the serialization logic. > > >> > > >> To avoid paying the allocation cost of adding a container object, > > >> singleton "accessor" objects are used to operate on the actual data. > See > > >> here for an example: > > >> https://gist.github.com/bdeggleston/52910225b817a8d54353125ca03f521d > > >> > > >> Mick and Robert Stupp have raised a few concerns, summarized below: > > >> > > >> 1. The patch is large (208 files / ~3.5k LOC) > > >> 2. Concerns about impact on stability > > >> 3. Parameterizing cell/clustering value types in this way makes > > >> ClassCastExceptions possible. > > >> 4. implications of feature freeze > > >> > > >> The patch is large, but the vast majority of it is adding type > > parameters > > >> to things. The changes here are wide, but not deep. The most complex > > parts > > >> are the collection serializers and other places where we're now having > > to > > >> do offset bookkeeping. These should be carefully reviewed, but they > > >> shouldn't be too difficult to verify and I've added some randomized > > tests > > >> to check them against a wide range of schemas. I'll also run some diff > > >> tests against clusters internally. > > >> > > >> Parameterizing cell and clustering values does make > ClassCastExceptions > > >> possible, but java's type system guards against this for the most > part. > > >> Regarding the feature freeze, I don't think it applies to performance > > >> improvements. > > >> > > >> Back to the point about stability though: in pracice, compaction gc > is a > > >> major contributor to cluster instability. In my experience, about 30% > of > > >> availability issues are gc related. Also, compaction gc tends to be > the > > >> limiting factor for repair, host replacements, and other topology > > changes, > > >> which limits how quickly you can recover from other issues. So the > patch > > >> does add some risk, but I think it's a net win for stability. > > >> > > >> Thoughts? > > >> - > > >> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > > >> For additional commands, e-mail: dev-h...@cassandra.apache.org > > >> > > >> > > > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > > For additional commands, e-mail: dev-h...@cassandra.apache.org > > > > >
