Re: Low hanging fruit crew

[Jeremy Hanna]

It sounds like everyone is on the same page - there won’t be a rubber stamp.  
It’s just to have a concerted effort to help these tickets move along as 
they’re often the first experience that contributors have with the project.  
I’m sure many of you know of the ‘lhf' tag that’s out there with an associated 
Jira search [1] on the How to Contribute page [2].  In the Jira search, you can 
see several that are either “patch available” or “awaiting feedback” so that 
search a place to start.  As for something fancier like a dashboard, I 
currently can’t make a share-able dashboard as a contributor to the project.  
Perhaps committers have more permissions there.

1) 
https://issues.apache.org/jira/issues/?jql=project%20%3D%2012310865%20AND%20labels%20%3D%20lhf%20AND%20status%20!%3D%20resolved
2) https://wiki.apache.org/cassandra/HowToContribute



> On Oct 19, 2016, at 5:27 PM, Jonathan Haddad  wrote:
> 
> Tagging tickets as LHF is a great idea.  There's plenty of people that
> would love to set up a JIRA dashboard saved search / nightly email for LHF
> tickets.
> 
> On Wed, Oct 19, 2016 at 1:34 PM Jeff Beck  wrote:
> 
>> Would it make sense to allow people submitting the patch to flag things as
>> LHF or small tasks? If it doesn't look like it is simple enough the team
>> could remove the label but it may help get feedback to patches more
>> quickly, even something saying accepted for review would be nice.
>> 
>> Personally if a ticket sits for a few weeks I have no idea what next steps
>> I should take to keep it moving forward. We may want to document what a
>> person submitting a patch should do next and if it is documented we should
>> add a link on
>> http://cassandra.apache.org/doc/latest/development/patches.html
>> 
>> Jeff
>> 
>> 
>> PS: My example is https://issues.apache.org/jira/browse/CASSANDRA-12633
>> 
>> On Wed, Oct 19, 2016 at 12:21 PM Edward Capriolo 
>> wrote:
>> 
>>> Also no one has said anything to the effect of 'we want to rubber stamp
>>> reviews' so that ...evil reason. Many of us are coders by trade and
>>> understand why that is bad.
>>> 
>>> On Wednesday, October 19, 2016, Edward Capriolo 
>>> wrote:
>>> 
 I realize that test passing a small tests and trivial reviews will not
 catch all issues. I am  not attempting to trivialize the review
>> process.
 
 Both deep and shallow bugs exist. The deep bugs, I am not convinced
>> that
 even an expert looking at the contribution for N days can account for a
 majority of them.
 
 The shallow ones may appear shallow and may be deep, but given that a
>> bug
 already exists an attempt to fix it usually does not arrive at
>> something
 worse.
 
 Many of these issues boil down to simple, seeemingly clear fixes. Some
>>> are
 just basic metric addition. Many have no interaction for weeks.
 
 
 On Wednesday, October 19, 2016, Jeff Jirsa > wrote:
 
> Let’s not get too far in the theoretical weeds. The email thread
>> really
> focused on low hanging tickets – tickets that need review, but
>>> definitely
> not 8099 level reviews:
> 
> 1) There’s a lot of low hanging tickets that would benefit from
>> outside
> contributors as their first-patch in Cassandra (like
> https://issues.apache.org/jira/browse/CASSANDRA-12541 ,
> https://issues.apache.org/jira/browse/CASSANDRA-12776 )
> 2) Some of these patches already exist and just need to be reviewed
>> and
> eventually committed.
> 
> Folks like Ed and Kurt have been really active in Jira lately, and
>> there
> aren’t a ton of people currently reviewing/committing – that’s part of
>>> OSS
> life, but the less friction that exists getting those patches reviewed
>>> and
> committed, the more people will be willing to contribute in the
>> future,
>>> and
> the better off the project will be.
> 
> 
> On 10/19/16, 9:14 AM, "Jeremy Hanna" 
>>> wrote:
> 
>> And just to be clear, I think everyone would welcome more testing for
> both regressions of new code correctness.  I think everyone would
> appreciate the time savings around more automation.  That should give
>>> more
> time for a thoughtful review - which is likely what new contributors
>>> really
> need to get familiar with different parts of the codebase.  These LHF
> reviews won’t be like the code reviews of the vnode or the 8099 ticket
> obviously, so it won’t be a huge burden.  But it has some very
>> tangible
> benefits imo, as has been said.
>> 
>>> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis 
> wrote:
>>> 
>>> I specifically used the phrase "problems that the test would not"
>> to
> show I
>>> am talking about more than 

Re: Low hanging fruit crew

[Jonathan Haddad]

Tagging tickets as LHF is a great idea.  There's plenty of people that
would love to set up a JIRA dashboard saved search / nightly email for LHF
tickets.

On Wed, Oct 19, 2016 at 1:34 PM Jeff Beck  wrote:

> Would it make sense to allow people submitting the patch to flag things as
> LHF or small tasks? If it doesn't look like it is simple enough the team
> could remove the label but it may help get feedback to patches more
> quickly, even something saying accepted for review would be nice.
>
> Personally if a ticket sits for a few weeks I have no idea what next steps
> I should take to keep it moving forward. We may want to document what a
> person submitting a patch should do next and if it is documented we should
> add a link on
> http://cassandra.apache.org/doc/latest/development/patches.html
>
> Jeff
>
>
> PS: My example is https://issues.apache.org/jira/browse/CASSANDRA-12633
>
> On Wed, Oct 19, 2016 at 12:21 PM Edward Capriolo 
> wrote:
>
> > Also no one has said anything to the effect of 'we want to rubber stamp
> > reviews' so that ...evil reason. Many of us are coders by trade and
> > understand why that is bad.
> >
> > On Wednesday, October 19, 2016, Edward Capriolo 
> > wrote:
> >
> > > I realize that test passing a small tests and trivial reviews will not
> > > catch all issues. I am  not attempting to trivialize the review
> process.
> > >
> > > Both deep and shallow bugs exist. The deep bugs, I am not convinced
> that
> > > even an expert looking at the contribution for N days can account for a
> > > majority of them.
> > >
> > > The shallow ones may appear shallow and may be deep, but given that a
> bug
> > > already exists an attempt to fix it usually does not arrive at
> something
> > > worse.
> > >
> > > Many of these issues boil down to simple, seeemingly clear fixes. Some
> > are
> > > just basic metric addition. Many have no interaction for weeks.
> > >
> > >
> > > On Wednesday, October 19, 2016, Jeff Jirsa  > > > wrote:
> > >
> > >> Let’s not get too far in the theoretical weeds. The email thread
> really
> > >> focused on low hanging tickets – tickets that need review, but
> > definitely
> > >> not 8099 level reviews:
> > >>
> > >> 1) There’s a lot of low hanging tickets that would benefit from
> outside
> > >> contributors as their first-patch in Cassandra (like
> > >> https://issues.apache.org/jira/browse/CASSANDRA-12541 ,
> > >> https://issues.apache.org/jira/browse/CASSANDRA-12776 )
> > >> 2) Some of these patches already exist and just need to be reviewed
> and
> > >> eventually committed.
> > >>
> > >> Folks like Ed and Kurt have been really active in Jira lately, and
> there
> > >> aren’t a ton of people currently reviewing/committing – that’s part of
> > OSS
> > >> life, but the less friction that exists getting those patches reviewed
> > and
> > >> committed, the more people will be willing to contribute in the
> future,
> > and
> > >> the better off the project will be.
> > >>
> > >>
> > >> On 10/19/16, 9:14 AM, "Jeremy Hanna" 
> > wrote:
> > >>
> > >> >And just to be clear, I think everyone would welcome more testing for
> > >> both regressions of new code correctness.  I think everyone would
> > >> appreciate the time savings around more automation.  That should give
> > more
> > >> time for a thoughtful review - which is likely what new contributors
> > really
> > >> need to get familiar with different parts of the codebase.  These LHF
> > >> reviews won’t be like the code reviews of the vnode or the 8099 ticket
> > >> obviously, so it won’t be a huge burden.  But it has some very
> tangible
> > >> benefits imo, as has been said.
> > >> >
> > >> >> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis 
> > >> wrote:
> > >> >>
> > >> >> I specifically used the phrase "problems that the test would not"
> to
> > >> show I
> > >> >> am talking about more than mechanical correctness.  Even if the
> tests
> > >> are
> > >> >> perfect (and as Jeremiah points out, how will you know that without
> > >> reading
> > >> >> the code?), you can still pass tests with bad code.  And is
> expecting
> > >> >> perfect tests really realistic for multithreaded code?
> > >> >>
> > >> >> But besides correctness, reviews should deal with
> > >> >>
> > >> >> 1. Efficiency.  Is there a quadratic loop that will blow up when
> the
> > >> number
> > >> >> of nodes in a cluster gets large?  Is there a linear amount of
> memory
> > >> used
> > >> >> that will cause problems when a partition gets large?  These are
> not
> > >> >> theoretical problems.
> > >> >>
> > >> >> 2. Maintainability.  Is this the simplest way to accomplish your
> > >> goals?  Is
> > >> >> there a method in SSTableReader that would make your life easier if
> > you
> > >> >> refactored it a bit instead of reinventing it?  Does this reduce

Re: Batch read requests to one physical host?

[Nate McCall]

I see a few slightly different things here (equally valuable) in
conjunction with CASSANDRA-10414:
- Wanting a small number of specific, non-sequential rows out of the
same partition (this is common, IME) and grouping those
- Extending batch semantics to reads with the same understanding with
mutate that if you put different partitions in the same batch it will
be slow

(I think Eric's IN(..) sorta fits with either of those).

Interesting!

On Thu, Oct 20, 2016 at 4:26 AM, Tyler Hobbs  wrote:
> There's a similar ticket focusing on range reads and secondary index
> queries, but the work for these could be done together:
> https://issues.apache.org/jira/browse/CASSANDRA-10414
>
> On Tue, Oct 18, 2016 at 5:59 PM, Dikang Gu  wrote:
>
>> Hi there,
>>
>> We have couple use cases that are doing fanout read for their data, means
>> one single read request from client contains multiple keys which live on
>> different physical hosts. (I know it's not recommended way to access C*).
>>
>> Right now, on the coordinator, it will issue separate read commands even
>> though they will go to the same physical host, which I think is causing a
>> lot of overheads.
>>
>> I'm wondering is it valuable to provide a new read command, that
>> coordinator can batch the reads to one datanode, and send to it in one
>> message, and datanode will return the results for all keys belong to it?
>>
>> Any similar ideas before?
>>
>>
>> --
>> Dikang
>>
>
>
>
> --
> Tyler Hobbs
> DataStax 

[GitHub] cassandra pull request #77: Remove legacy docs

[eprothro]

Github user eprothro closed the pull request at:

https://github.com/apache/cassandra/pull/77


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cassandra pull request #77: 12804 3.x

[eprothro]

GitHub user eprothro opened a pull request:

https://github.com/apache/cassandra/pull/77

12804 3.x



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/eprothro/cassandra 12804-3.X

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cassandra/pull/77.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #77


commit 41c90d85fdb691639a7259f8510f1b2053f50d30
Author: Evan Prothro 
Date:   2016-10-19T18:25:12Z

Add clarity to process of building the docs

commit cedda54fdbe3c6e19498d2e2af75ce1de9a25167
Author: Evan Prothro 
Date:   2016-10-19T19:57:27Z

Remove legacy cql textile/html doc in favor of new sphinx docs




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Low hanging fruit crew

[Jeff Beck]

Would it make sense to allow people submitting the patch to flag things as
LHF or small tasks? If it doesn't look like it is simple enough the team
could remove the label but it may help get feedback to patches more
quickly, even something saying accepted for review would be nice.

Personally if a ticket sits for a few weeks I have no idea what next steps
I should take to keep it moving forward. We may want to document what a
person submitting a patch should do next and if it is documented we should
add a link on
http://cassandra.apache.org/doc/latest/development/patches.html

Jeff


PS: My example is https://issues.apache.org/jira/browse/CASSANDRA-12633

On Wed, Oct 19, 2016 at 12:21 PM Edward Capriolo 
wrote:

> Also no one has said anything to the effect of 'we want to rubber stamp
> reviews' so that ...evil reason. Many of us are coders by trade and
> understand why that is bad.
>
> On Wednesday, October 19, 2016, Edward Capriolo 
> wrote:
>
> > I realize that test passing a small tests and trivial reviews will not
> > catch all issues. I am  not attempting to trivialize the review process.
> >
> > Both deep and shallow bugs exist. The deep bugs, I am not convinced that
> > even an expert looking at the contribution for N days can account for a
> > majority of them.
> >
> > The shallow ones may appear shallow and may be deep, but given that a bug
> > already exists an attempt to fix it usually does not arrive at something
> > worse.
> >
> > Many of these issues boil down to simple, seeemingly clear fixes. Some
> are
> > just basic metric addition. Many have no interaction for weeks.
> >
> >
> > On Wednesday, October 19, 2016, Jeff Jirsa  > > wrote:
> >
> >> Let’s not get too far in the theoretical weeds. The email thread really
> >> focused on low hanging tickets – tickets that need review, but
> definitely
> >> not 8099 level reviews:
> >>
> >> 1) There’s a lot of low hanging tickets that would benefit from outside
> >> contributors as their first-patch in Cassandra (like
> >> https://issues.apache.org/jira/browse/CASSANDRA-12541 ,
> >> https://issues.apache.org/jira/browse/CASSANDRA-12776 )
> >> 2) Some of these patches already exist and just need to be reviewed and
> >> eventually committed.
> >>
> >> Folks like Ed and Kurt have been really active in Jira lately, and there
> >> aren’t a ton of people currently reviewing/committing – that’s part of
> OSS
> >> life, but the less friction that exists getting those patches reviewed
> and
> >> committed, the more people will be willing to contribute in the future,
> and
> >> the better off the project will be.
> >>
> >>
> >> On 10/19/16, 9:14 AM, "Jeremy Hanna" 
> wrote:
> >>
> >> >And just to be clear, I think everyone would welcome more testing for
> >> both regressions of new code correctness.  I think everyone would
> >> appreciate the time savings around more automation.  That should give
> more
> >> time for a thoughtful review - which is likely what new contributors
> really
> >> need to get familiar with different parts of the codebase.  These LHF
> >> reviews won’t be like the code reviews of the vnode or the 8099 ticket
> >> obviously, so it won’t be a huge burden.  But it has some very tangible
> >> benefits imo, as has been said.
> >> >
> >> >> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis 
> >> wrote:
> >> >>
> >> >> I specifically used the phrase "problems that the test would not" to
> >> show I
> >> >> am talking about more than mechanical correctness.  Even if the tests
> >> are
> >> >> perfect (and as Jeremiah points out, how will you know that without
> >> reading
> >> >> the code?), you can still pass tests with bad code.  And is expecting
> >> >> perfect tests really realistic for multithreaded code?
> >> >>
> >> >> But besides correctness, reviews should deal with
> >> >>
> >> >> 1. Efficiency.  Is there a quadratic loop that will blow up when the
> >> number
> >> >> of nodes in a cluster gets large?  Is there a linear amount of memory
> >> used
> >> >> that will cause problems when a partition gets large?  These are not
> >> >> theoretical problems.
> >> >>
> >> >> 2. Maintainability.  Is this the simplest way to accomplish your
> >> goals?  Is
> >> >> there a method in SSTableReader that would make your life easier if
> you
> >> >> refactored it a bit instead of reinventing it?  Does this reduce
> >> technical
> >> >> debt or add to it?
> >> >>
> >> >> 3. "Bus factor."  If only the author understands the code and all
> >> anyone
> >> >> else knows is that tests pass, the project will quickly be in bad
> >> shape.
> >> >> Review should ensure that at least one other person understand the
> >> code,
> >> >> what it does, and why, at a level that s/he could fix bugs later in
> it
> >> if
> >> >> necessary.
> >> >>
> >> >> On Wed, Oct 19, 2016 at 10:52 AM, 

[GitHub] cassandra pull request #76: CASSANDRA-12541, CASSANDRA-12542, CASSANDRA-1254...

[deshpamit]

Github user deshpamit closed the pull request at:

https://github.com/apache/cassandra/pull/76


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re[2]: Histogram error "Unable to compute ceiling for max when histogram overflowed"

[Владимир Бухтояров]

The null(zero) values of snapshot are useless for problem analysing, because it 
is impossible to distinguishing case when there are no events from case when 
events were dispatched too slow. I do not see any criminal to return  999-th 
percentile as 3h when histogram configured with 3h max and any latency is 4h.


Best regards,
Bukhtoyarov Vladimir
email jseco...@mail.ru
skype live:fanat-tdd
Github: https://github.com/vladimir-bukhtoyarov
mobile +79618096798

>Среда, 19 октября 2016, 20:17 +03:00 от Ken Hancock :
>
>I would suggest metrics should return null values instead of false values.
>
>On Wed, Oct 19, 2016 at 12:21 PM, Владимир Бухтояров <
>jseco...@mail.ru.invalid > wrote:
>
>>
>> Hi to all,
>>
>> I want to fix  https://issues.apache.org/jira/browse/CASSANDRA-11063
>> This issue is very ugly for me, because when something works slow then it
>> is impossible to capture metrics and save it to monitoring database for
>> future investigation. Moreover when one histogram throw exception then many
>> metrics-exporters are unable to export metrics for whole MetricRegistry(for
>> example MetricsServlet), so when overflow happen in one histogram then I
>> have no history data at all.
>>
>> I propose to implement the following changes:
>> 1. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will
>> return maximum trackable value instead of Long.MAX_VALUE
>> 2. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will
>> never throw IllegalStateException, instead, it will use maximum trackable
>> value as regular value in percentile and average calculation.
>> 3.  If anybody want to save old behavior(prefer to crash instead of
>> inaccurate reporting) then I can add configuration parameter to save
>> previous behavior, moreover I can leave old behavior as default, for my
>> needs it will be enough to have some option to avoid crashes.
>>
>>
>> Best regards,
>> Bukhtoyarov Vladimir
>> email  jseco...@mail.ru
>> skype live:fanat-tdd
>> Github:  https://github.com/vladimir-bukhtoyarov
>>

Re: Histogram error "Unable to compute ceiling for max when histogram overflowed"

[Ken Hancock]

I would suggest metrics should return null values instead of false values.

On Wed, Oct 19, 2016 at 12:21 PM, Владимир Бухтояров <
jseco...@mail.ru.invalid> wrote:

>
> Hi to all,
>
> I want to fix  https://issues.apache.org/jira/browse/CASSANDRA-11063
> This issue is very ugly for me, because when something works slow then it
> is impossible to capture metrics and save it to monitoring database for
> future investigation. Moreover when one histogram throw exception then many
> metrics-exporters are unable to export metrics for whole MetricRegistry(for
> example MetricsServlet), so when overflow happen in one histogram then I
> have no history data at all.
>
> I propose to implement the following changes:
> 1. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will
> return maximum trackable value instead of Long.MAX_VALUE
> 2. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will
> never throw IllegalStateException, instead, it will use maximum trackable
> value as regular value in percentile and average calculation.
> 3.  If anybody want to save old behavior(prefer to crash instead of
> inaccurate reporting) then I can add configuration parameter to save
> previous behavior, moreover I can leave old behavior as default, for my
> needs it will be enough to have some option to avoid crashes.
>
>
> Best regards,
> Bukhtoyarov Vladimir
> email jseco...@mail.ru
> skype live:fanat-tdd
> Github: https://github.com/vladimir-bukhtoyarov
>

Re: Low hanging fruit crew

[Edward Capriolo]

I realize that test passing a small tests and trivial reviews will not
catch all issues. I am  not attempting to trivialize the review process.

Both deep and shallow bugs exist. The deep bugs, I am not convinced that
even an expert looking at the contribution for N days can account for a
majority of them.

The shallow ones may appear shallow and may be deep, but given that a bug
already exists an attempt to fix it usually does not arrive at something
worse.

Many of these issues boil down to simple, seeemingly clear fixes. Some are
just basic metric addition. Many have no interaction for weeks.


On Wednesday, October 19, 2016, Jeff Jirsa 
wrote:

> Let’s not get too far in the theoretical weeds. The email thread really
> focused on low hanging tickets – tickets that need review, but definitely
> not 8099 level reviews:
>
> 1) There’s a lot of low hanging tickets that would benefit from outside
> contributors as their first-patch in Cassandra (like
> https://issues.apache.org/jira/browse/CASSANDRA-12541 ,
> https://issues.apache.org/jira/browse/CASSANDRA-12776 )
> 2) Some of these patches already exist and just need to be reviewed and
> eventually committed.
>
> Folks like Ed and Kurt have been really active in Jira lately, and there
> aren’t a ton of people currently reviewing/committing – that’s part of OSS
> life, but the less friction that exists getting those patches reviewed and
> committed, the more people will be willing to contribute in the future, and
> the better off the project will be.
>
>
> On 10/19/16, 9:14 AM, "Jeremy Hanna"  > wrote:
>
> >And just to be clear, I think everyone would welcome more testing for
> both regressions of new code correctness.  I think everyone would
> appreciate the time savings around more automation.  That should give more
> time for a thoughtful review - which is likely what new contributors really
> need to get familiar with different parts of the codebase.  These LHF
> reviews won’t be like the code reviews of the vnode or the 8099 ticket
> obviously, so it won’t be a huge burden.  But it has some very tangible
> benefits imo, as has been said.
> >
> >> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis  > wrote:
> >>
> >> I specifically used the phrase "problems that the test would not" to
> show I
> >> am talking about more than mechanical correctness.  Even if the tests
> are
> >> perfect (and as Jeremiah points out, how will you know that without
> reading
> >> the code?), you can still pass tests with bad code.  And is expecting
> >> perfect tests really realistic for multithreaded code?
> >>
> >> But besides correctness, reviews should deal with
> >>
> >> 1. Efficiency.  Is there a quadratic loop that will blow up when the
> number
> >> of nodes in a cluster gets large?  Is there a linear amount of memory
> used
> >> that will cause problems when a partition gets large?  These are not
> >> theoretical problems.
> >>
> >> 2. Maintainability.  Is this the simplest way to accomplish your
> goals?  Is
> >> there a method in SSTableReader that would make your life easier if you
> >> refactored it a bit instead of reinventing it?  Does this reduce
> technical
> >> debt or add to it?
> >>
> >> 3. "Bus factor."  If only the author understands the code and all anyone
> >> else knows is that tests pass, the project will quickly be in bad shape.
> >> Review should ensure that at least one other person understand the code,
> >> what it does, and why, at a level that s/he could fix bugs later in it
> if
> >> necessary.
> >>
> >> On Wed, Oct 19, 2016 at 10:52 AM, Jonathan Haddad  > wrote:
> >>
> >>> Shouldn't the tests test the code for correctness?
> >>>
> >>> On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  > wrote:
> >>>
>  On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
>  benjamin.le...@datastax.com 
> > wrote:
> 
> > Having the test passing does not mean that a patch is fine. Which is
> >>> why
>  we
> > have a review check list.
> > I never put a patch available without having the tests passing but
> most
>  of
> > my patches never pass on the first try. We always make mistakes no
> >>> matter
> > how hard we try.
> > The reviewer job is to catch those mistakes by looking at the patch
> >>> from
> > another angle. Of course, sometime, both of them fail.
> >
> 
>  Agreed.  Review should not just be a "tests pass, +1" rubber stamp,
> but
>  actually checking the code for correctness.  The former is just
> process;
>  the latter actually catches problems that the tests would not.  (And
> this
>  is true even if the tests are much much better than ours.)
> 
>  --
>  Jonathan Ellis
>  co-founder, https://urldefense.proofpoint.com/v2/url?u=http-3A__www.
> 

[GitHub] cassandra issue #76: CASSANDRA-12541, CASSANDRA-12542, CASSANDRA-12543 and C...

[jeffjirsa]

Github user jeffjirsa commented on the issue:

https://github.com/apache/cassandra/pull/76
  
@deshpamit - Thanks so much for your patch. Two points:

1) The project uses JIRA for patches and issue tracking, not github issues 
/ pull requests. We'll try to get patches wherever they arrive, but for 
consistency, posting on the JIRA is more likely to be seen.
2) You're missing a close paren here: 
https://github.com/apache/cassandra/pull/76/files#diff-8d033ba4918a0c6a3781e675678f3ac3R196

I encourage you to review 
http://cassandra.apache.org/doc/latest/development/patches.html as well - we'll 
do our best to accept patches as quickly as we can, but if they match the 
expected format, it's much easier for committers to review and commit. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Low hanging fruit crew

[Jeff Jirsa]

Let’s not get too far in the theoretical weeds. The email thread really focused 
on low hanging tickets – tickets that need review, but definitely not 8099 
level reviews:

1) There’s a lot of low hanging tickets that would benefit from outside 
contributors as their first-patch in Cassandra (like 
https://issues.apache.org/jira/browse/CASSANDRA-12541 , 
https://issues.apache.org/jira/browse/CASSANDRA-12776 )
2) Some of these patches already exist and just need to be reviewed and 
eventually committed. 

Folks like Ed and Kurt have been really active in Jira lately, and there aren’t 
a ton of people currently reviewing/committing – that’s part of OSS life, but 
the less friction that exists getting those patches reviewed and committed, the 
more people will be willing to contribute in the future, and the better off the 
project will be. 


On 10/19/16, 9:14 AM, "Jeremy Hanna"  wrote:

>And just to be clear, I think everyone would welcome more testing for both 
>regressions of new code correctness.  I think everyone would appreciate the 
>time savings around more automation.  That should give more time for a 
>thoughtful review - which is likely what new contributors really need to get 
>familiar with different parts of the codebase.  These LHF reviews won’t be 
>like the code reviews of the vnode or the 8099 ticket obviously, so it won’t 
>be a huge burden.  But it has some very tangible benefits imo, as has been 
>said.
>
>> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis  wrote:
>> 
>> I specifically used the phrase "problems that the test would not" to show I
>> am talking about more than mechanical correctness.  Even if the tests are
>> perfect (and as Jeremiah points out, how will you know that without reading
>> the code?), you can still pass tests with bad code.  And is expecting
>> perfect tests really realistic for multithreaded code?
>> 
>> But besides correctness, reviews should deal with
>> 
>> 1. Efficiency.  Is there a quadratic loop that will blow up when the number
>> of nodes in a cluster gets large?  Is there a linear amount of memory used
>> that will cause problems when a partition gets large?  These are not
>> theoretical problems.
>> 
>> 2. Maintainability.  Is this the simplest way to accomplish your goals?  Is
>> there a method in SSTableReader that would make your life easier if you
>> refactored it a bit instead of reinventing it?  Does this reduce technical
>> debt or add to it?
>> 
>> 3. "Bus factor."  If only the author understands the code and all anyone
>> else knows is that tests pass, the project will quickly be in bad shape.
>> Review should ensure that at least one other person understand the code,
>> what it does, and why, at a level that s/he could fix bugs later in it if
>> necessary.
>> 
>> On Wed, Oct 19, 2016 at 10:52 AM, Jonathan Haddad  wrote:
>> 
>>> Shouldn't the tests test the code for correctness?
>>> 
>>> On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  wrote:
>>> 
 On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
 benjamin.le...@datastax.com
> wrote:
 
> Having the test passing does not mean that a patch is fine. Which is
>>> why
 we
> have a review check list.
> I never put a patch available without having the tests passing but most
 of
> my patches never pass on the first try. We always make mistakes no
>>> matter
> how hard we try.
> The reviewer job is to catch those mistakes by looking at the patch
>>> from
> another angle. Of course, sometime, both of them fail.
> 
 
 Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
 actually checking the code for correctness.  The former is just process;
 the latter actually catches problems that the tests would not.  (And this
 is true even if the tests are much much better than ours.)
 
 --
 Jonathan Ellis
 co-founder, 
 https://urldefense.proofpoint.com/v2/url?u=http-3A__www.datastax.com=DQIFaQ=08AGY6txKsvMOP6lYkHQpPMRA1U6kqhAwGa8-0QCg3M=yfYEBHVkX6l0zImlOIBID0gmhluYPD5Jje-3CtaT3ow=eXI0TPp0DM06kmTuJQRNcUX5zy_O_KhWcDKMA-jxww0=D28xk3JpCIOAQnCXJAVky0lJJv_mPnYwy4gKxLKsSqw=
  
 @spyced
 
>>> 
>> 
>> 
>> 
>> -- 
>> Jonathan Ellis
>> co-founder, 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.datastax.com=DQIFaQ=08AGY6txKsvMOP6lYkHQpPMRA1U6kqhAwGa8-0QCg3M=yfYEBHVkX6l0zImlOIBID0gmhluYPD5Jje-3CtaT3ow=eXI0TPp0DM06kmTuJQRNcUX5zy_O_KhWcDKMA-jxww0=D28xk3JpCIOAQnCXJAVky0lJJv_mPnYwy4gKxLKsSqw=
>>  
>> @spyced
>


CONFIDENTIALITY NOTE: This e-mail and any attachments are confidential and may 
be legally privileged. If you are not the intended recipient, do not disclose, 
copy, distribute, or use this email or any attachments. If you have received 
this in error please let the sender know and then delete the email and all 
attachments.

Histogram error "Unable to compute ceiling for max when histogram overflowed"

[Владимир Бухтояров]

Hi to all,

I want to fix  https://issues.apache.org/jira/browse/CASSANDRA-11063
This issue is very ugly for me, because when something works slow then it is 
impossible to capture metrics and save it to monitoring database for future 
investigation. Moreover when one histogram throw exception then many 
metrics-exporters are unable to export metrics for whole MetricRegistry(for 
example MetricsServlet), so when overflow happen in one histogram then I have 
no history data at all.

I propose to implement the following changes: 
1. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will return 
maximum trackable value instead of Long.MAX_VALUE
2. The DecayingEstimatedHistogramReservoir and EstimatedHistogram will never 
throw IllegalStateException, instead, it will use maximum trackable value as 
regular value in percentile and average calculation.
3.  If anybody want to save old behavior(prefer to crash instead of inaccurate 
reporting) then I can add configuration parameter to save previous behavior, 
moreover I can leave old behavior as default, for my needs it will be enough to 
have some option to avoid crashes.


Best regards,
Bukhtoyarov Vladimir
email jseco...@mail.ru
skype live:fanat-tdd
Github: https://github.com/vladimir-bukhtoyarov

Re: Low hanging fruit crew

[Jeremy Hanna]

And just to be clear, I think everyone would welcome more testing for both 
regressions of new code correctness.  I think everyone would appreciate the 
time savings around more automation.  That should give more time for a 
thoughtful review - which is likely what new contributors really need to get 
familiar with different parts of the codebase.  These LHF reviews won’t be like 
the code reviews of the vnode or the 8099 ticket obviously, so it won’t be a 
huge burden.  But it has some very tangible benefits imo, as has been said.

> On Oct 19, 2016, at 11:08 AM, Jonathan Ellis  wrote:
> 
> I specifically used the phrase "problems that the test would not" to show I
> am talking about more than mechanical correctness.  Even if the tests are
> perfect (and as Jeremiah points out, how will you know that without reading
> the code?), you can still pass tests with bad code.  And is expecting
> perfect tests really realistic for multithreaded code?
> 
> But besides correctness, reviews should deal with
> 
> 1. Efficiency.  Is there a quadratic loop that will blow up when the number
> of nodes in a cluster gets large?  Is there a linear amount of memory used
> that will cause problems when a partition gets large?  These are not
> theoretical problems.
> 
> 2. Maintainability.  Is this the simplest way to accomplish your goals?  Is
> there a method in SSTableReader that would make your life easier if you
> refactored it a bit instead of reinventing it?  Does this reduce technical
> debt or add to it?
> 
> 3. "Bus factor."  If only the author understands the code and all anyone
> else knows is that tests pass, the project will quickly be in bad shape.
> Review should ensure that at least one other person understand the code,
> what it does, and why, at a level that s/he could fix bugs later in it if
> necessary.
> 
> On Wed, Oct 19, 2016 at 10:52 AM, Jonathan Haddad  wrote:
> 
>> Shouldn't the tests test the code for correctness?
>> 
>> On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  wrote:
>> 
>>> On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
>>> benjamin.le...@datastax.com
 wrote:
>>> 
 Having the test passing does not mean that a patch is fine. Which is
>> why
>>> we
 have a review check list.
 I never put a patch available without having the tests passing but most
>>> of
 my patches never pass on the first try. We always make mistakes no
>> matter
 how hard we try.
 The reviewer job is to catch those mistakes by looking at the patch
>> from
 another angle. Of course, sometime, both of them fail.
 
>>> 
>>> Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
>>> actually checking the code for correctness.  The former is just process;
>>> the latter actually catches problems that the tests would not.  (And this
>>> is true even if the tests are much much better than ours.)
>>> 
>>> --
>>> Jonathan Ellis
>>> co-founder, http://www.datastax.com
>>> @spyced
>>> 
>> 
> 
> 
> 
> -- 
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced

Re: Low hanging fruit crew

[Jonathan Ellis]

I specifically used the phrase "problems that the test would not" to show I
am talking about more than mechanical correctness.  Even if the tests are
perfect (and as Jeremiah points out, how will you know that without reading
the code?), you can still pass tests with bad code.  And is expecting
perfect tests really realistic for multithreaded code?

But besides correctness, reviews should deal with

1. Efficiency.  Is there a quadratic loop that will blow up when the number
of nodes in a cluster gets large?  Is there a linear amount of memory used
that will cause problems when a partition gets large?  These are not
theoretical problems.

2. Maintainability.  Is this the simplest way to accomplish your goals?  Is
there a method in SSTableReader that would make your life easier if you
refactored it a bit instead of reinventing it?  Does this reduce technical
debt or add to it?

3. "Bus factor."  If only the author understands the code and all anyone
else knows is that tests pass, the project will quickly be in bad shape.
Review should ensure that at least one other person understand the code,
what it does, and why, at a level that s/he could fix bugs later in it if
necessary.

On Wed, Oct 19, 2016 at 10:52 AM, Jonathan Haddad  wrote:

> Shouldn't the tests test the code for correctness?
>
> On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  wrote:
>
> > On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
> > benjamin.le...@datastax.com
> > > wrote:
> >
> > > Having the test passing does not mean that a patch is fine. Which is
> why
> > we
> > > have a review check list.
> > > I never put a patch available without having the tests passing but most
> > of
> > > my patches never pass on the first try. We always make mistakes no
> matter
> > > how hard we try.
> > > The reviewer job is to catch those mistakes by looking at the patch
> from
> > > another angle. Of course, sometime, both of them fail.
> > >
> >
> > Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
> > actually checking the code for correctness.  The former is just process;
> > the latter actually catches problems that the tests would not.  (And this
> > is true even if the tests are much much better than ours.)
> >
> > --
> > Jonathan Ellis
> > co-founder, http://www.datastax.com
> > @spyced
> >
>



-- 
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced

Re: Low hanging fruit crew

[Jeremiah D Jordan]

Unless the reviewer reviews the for content, then you don’t know if they do or 
not.

-Jeremiah

> On Oct 19, 2016, at 10:52 AM, Jonathan Haddad  wrote:
> 
> Shouldn't the tests test the code for correctness?
> 
> On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  wrote:
> 
>> On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
>> benjamin.le...@datastax.com
>>> wrote:
>> 
>>> Having the test passing does not mean that a patch is fine. Which is why
>> we
>>> have a review check list.
>>> I never put a patch available without having the tests passing but most
>> of
>>> my patches never pass on the first try. We always make mistakes no matter
>>> how hard we try.
>>> The reviewer job is to catch those mistakes by looking at the patch from
>>> another angle. Of course, sometime, both of them fail.
>>> 
>> 
>> Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
>> actually checking the code for correctness.  The former is just process;
>> the latter actually catches problems that the tests would not.  (And this
>> is true even if the tests are much much better than ours.)
>> 
>> --
>> Jonathan Ellis
>> co-founder, http://www.datastax.com
>> @spyced
>> 

Re: Low hanging fruit crew

[Jonathan Haddad]

Shouldn't the tests test the code for correctness?

On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis  wrote:

> On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <
> benjamin.le...@datastax.com
> > wrote:
>
> > Having the test passing does not mean that a patch is fine. Which is why
> we
> > have a review check list.
> > I never put a patch available without having the tests passing but most
> of
> > my patches never pass on the first try. We always make mistakes no matter
> > how hard we try.
> > The reviewer job is to catch those mistakes by looking at the patch from
> > another angle. Of course, sometime, both of them fail.
> >
>
> Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
> actually checking the code for correctness.  The former is just process;
> the latter actually catches problems that the tests would not.  (And this
> is true even if the tests are much much better than ours.)
>
> --
> Jonathan Ellis
> co-founder, http://www.datastax.com
> @spyced
>

Re: Low hanging fruit crew

[Jonathan Ellis]

On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer  wrote:

> Having the test passing does not mean that a patch is fine. Which is why we
> have a review check list.
> I never put a patch available without having the tests passing but most of
> my patches never pass on the first try. We always make mistakes no matter
> how hard we try.
> The reviewer job is to catch those mistakes by looking at the patch from
> another angle. Of course, sometime, both of them fail.
>

Agreed.  Review should not just be a "tests pass, +1" rubber stamp, but
actually checking the code for correctness.  The former is just process;
the latter actually catches problems that the tests would not.  (And this
is true even if the tests are much much better than ours.)

-- 
Jonathan Ellis
co-founder, http://www.datastax.com
@spyced

Re: Batch read requests to one physical host?

[Tyler Hobbs]

There's a similar ticket focusing on range reads and secondary index
queries, but the work for these could be done together:
https://issues.apache.org/jira/browse/CASSANDRA-10414

On Tue, Oct 18, 2016 at 5:59 PM, Dikang Gu  wrote:

> Hi there,
>
> We have couple use cases that are doing fanout read for their data, means
> one single read request from client contains multiple keys which live on
> different physical hosts. (I know it's not recommended way to access C*).
>
> Right now, on the coordinator, it will issue separate read commands even
> though they will go to the same physical host, which I think is causing a
> lot of overheads.
>
> I'm wondering is it valuable to provide a new read command, that
> coordinator can batch the reads to one datanode, and send to it in one
> message, and datanode will return the results for all keys belong to it?
>
> Any similar ideas before?
>
>
> --
> Dikang
>



-- 
Tyler Hobbs
DataStax 

Re: Low hanging fruit crew

[Benjamin Lerer]

Having the test passing does not mean that a patch is fine. Which is why we
have a review check list.
I never put a patch available without having the tests passing but most of
my patches never pass on the first try. We always make mistakes no matter
how hard we try.
The reviewer job is to catch those mistakes by looking at the patch from
another angle. Of course, sometime, both of them fail.


On Wed, Oct 19, 2016 at 3:01 PM, Edward Capriolo 
wrote:

> Yes. The LHFC crew should always pay it forward. Not many of us have a
> super computer to run all the tests, but for things that are out there
> marked patch_available apply it to see that it applies clean, if it
> includes a test run that test (and possibly some related ones in the
> file/folder etc for quick coverage). A nice initial sweep is a good thing.
>
> I have seen before a process which triggered and auto-build when the patch
> was added to the ticket. This took a burden off the committers, by the time
> someone got to the ticket they already knew if tests passed then it was
> only a style and fine tuning review.
>
> In this case if we have a good initial pass we can hopefully speed along
> the process.
>
> On Wed, Oct 19, 2016 at 4:18 AM, kurt Greaves 
> wrote:
>
> > On 19 October 2016 at 05:30, Nate McCall  wrote:
> >
> > > if you are offering up resources for review and test coverage,
> > > there is work out there. Most immediately in the department of reviews
> > > for "Patch Available."
> > >
> >
> > We can certainly put some minds to this. There are a few of us with a
> very
> > good understanding of working Cassandra yet could use more exposure to
> the
> > code base. We'll start getting out there and looking for things to
> review.
> >
> > Kurt Greaves
> > k...@instaclustr.com
> > www.instaclustr.com
> >
>

Re: Low hanging fruit crew

[Edward Capriolo]

Yes. The LHFC crew should always pay it forward. Not many of us have a
super computer to run all the tests, but for things that are out there
marked patch_available apply it to see that it applies clean, if it
includes a test run that test (and possibly some related ones in the
file/folder etc for quick coverage). A nice initial sweep is a good thing.

I have seen before a process which triggered and auto-build when the patch
was added to the ticket. This took a burden off the committers, by the time
someone got to the ticket they already knew if tests passed then it was
only a style and fine tuning review.

In this case if we have a good initial pass we can hopefully speed along
the process.

On Wed, Oct 19, 2016 at 4:18 AM, kurt Greaves  wrote:

> On 19 October 2016 at 05:30, Nate McCall  wrote:
>
> > if you are offering up resources for review and test coverage,
> > there is work out there. Most immediately in the department of reviews
> > for "Patch Available."
> >
>
> We can certainly put some minds to this. There are a few of us with a very
> good understanding of working Cassandra yet could use more exposure to the
> code base. We'll start getting out there and looking for things to review.
>
> Kurt Greaves
> k...@instaclustr.com
> www.instaclustr.com
>

Re: Low hanging fruit crew

[kurt Greaves]

On 19 October 2016 at 05:30, Nate McCall  wrote:

> if you are offering up resources for review and test coverage,
> there is work out there. Most immediately in the department of reviews
> for "Patch Available."
>

We can certainly put some minds to this. There are a few of us with a very
good understanding of working Cassandra yet could use more exposure to the
code base. We'll start getting out there and looking for things to review.

Kurt Greaves
k...@instaclustr.com
www.instaclustr.com

Re: 8099 Storage Format Documentation as used with PRIMARY_INDEX

[Michael Kjellman]

Ugh, just finally figured the "header" bit of my question out. Mega lame. :\

> On Oct 18, 2016, at 9:17 AM, Michael Kjellman  
> wrote:
> 
> I'm working on writing Birch for trunk and I noticed the following:
> 
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java#L503
> 
> Prior to 3.0 the offset was the literal offset into the data file, yet now we 
> seem to be doing the position encoded with the key (for all rows regardless 
> of if they're > 64kb and thus have an index component) plus the serialized 
> offset. I also see there is now a a "header" offset.
> 
> In RowIndexEntry there is:
> 
> 
> /**
> * @return the offset to the start of the header information for this row.
> * For some formats this may not be the start of the row.
> */
> public long headerOffset()
> {
>return 0;
> }
> 
> /**
> * The length of the row header (partition key, partition deletion and static 
> row).
> * This value is only provided for indexed entries and this method will throw
> * {@code UnsupportedOperationException} if {@code !isIndexed()}.
> */
> public long headerLength()
> {
>throw new UnsupportedOperationException();
> }
> 
> 
> In 2.1 we stored the partition key, deletion, but not static row -- but we 
> didn't need or use this so I'm guessing this is actually just to support 
> static rows? Is there any further documentation around the header in other 
> classes that I just haven't come across yet? Any thoughts on position + 
> offset and why this behavior changed? Thanks
> 
> best,
> kjellman