Re: Flaky tests

[Roman Leventov]

In this PR: https://github.com/apache/incubator-druid/pull/5957
(unrelatedly) I made several changes that should hopefully help to analyze
flaky failures, e. g. printing deadlocked threads (if any) if a test times
out. Also printing full stack traces on failures.

On Fri, 7 Sep 2018 at 00:28, Gian Merlino  wrote:

> Our CI has been super flaky lately: it seems rare that a PR is able to pass
> without a few retries. In an effort to try to help I added a new label
> "Flaky tests" and tagged all the open issues that look related to flaky
> tests. I also closed a few that have been open for a long time and I don't
> recall seeing in a while. They are all here:
>
> https://github.com/apache/incubator-druid/labels/Flaky%20test
>
> In a few cases I edited the titles so they all have the specific test case
> that failed (the class and method). I think it helps to have one issue per
> method, that way we can track them separately.
>
> Non-scientifically I seem to be noticing these four often these days:
>
> 1) https://github.com/apache/incubator-druid/issues/6296
> 2) https://github.com/apache/incubator-druid/issues/2373
> 3) https://github.com/apache/incubator-druid/issues/6311
> 4) https://github.com/apache/incubator-druid/issues/6312
>
> Please, if we can, let's spend some time looking into what is going on with
> these tests. We will thank ourselves when it makes PR flow smoother!
>

TODOs for documentation

[Roman Leventov]

Druid project has a policy of creating issues on Github instead of TODOs in
code, but I feel that this is an overkill for small documentation-only
tasks. I suggest to allow TODOs that ask just for writing or rewriting some
text (including internal Javadoc documentation), but not development of any
logic, code experiments, benchmarking, etc.

The question is arisen here:
https://github.com/apache/incubator-druid/pull/6370#discussion_r219694721

'Design Review' tag

[Roman Leventov]

Recently Charles (as far as I remember) asked what is the criteria for
giving a PR 'Design Review' tag.

I suggest that *a PR should be labelled "Design Review" if it will be hard
to undo it (after it appears in some release), it will have lasting
consequences on the codebase.*

Addition of a new config property, public API, or changing existing public
API is always hard to undo because it immediately becomes a subject for
backwards compatibility. But some large code changes, even without
user-facing API or config changes, could have similar effect on the
codebase.

Please keep in mind that it's not just for requiring two reviews instead of
one, but also for really reviewing the design. E. g. bad config
key/property names could stay forever because it's really hard to rename
them, even harder than change Java APIs.

Metrics updates in Release Notes

[Roman Leventov]

It's suggested that the person that prepares Druid Release Notes (I think
it's Jon usually) goes through all PRs labelled "Area - Metrics/Event
Emitting" (
https://github.com/apache/incubator-druid/pulls?q=is%3Apr+sort%3Aupdated-desc+label%3A%22Area+-+Metrics%2FEvent+Emitting%22+is%3Aclosed+milestone%3A0.13.0)
along with "Release Notes", to present this information in the release
notes.

BTW I wonder is the a document in the repository or elsewhere that
describes the release process?

Re: Metrics updates in Release Notes

[Roman Leventov]

I think people will often forget to put both tags, so the person who does
the release should check the tag Metrics/Event Emitting anyway, just in
case.

On Wed, 31 Oct 2018 at 18:09, Gian Merlino  wrote:

> I don't think we have a doc about how to do a release, but yeah it would be
> great to have it. Dave, would you be able to put it together while you
> manage this release? I am sure it will differ substantially from what we've
> done in the past, because of the new Apache-ified stuff.
>
> On Wed, Oct 31, 2018 at 10:07 AM Gian Merlino  wrote:
>
> > Why not also tag those with "Release Notes"? It makes it a lot easier for
> > release managers to do their jobs if they just have to look at one label.
> > (Or two, I guess: "release notes" and "incompatible". But I would be down
> > to merge them.)
> >
> > On Wed, Oct 31, 2018 at 9:26 AM David Lim  wrote:
> >
> >> Thanks Roman. I'm helping with the release this time so I will check the
> >> PRs with that label and include them in the release notes as
> appropriate.
> >>
> >> As far as I know, there isn't any document like that, but I agree it
> would
> >> be quite useful.
> >>
> >> On Wed, Oct 31, 2018 at 9:04 AM Roman Leventov 
> >> wrote:
> >>
> >> > It's suggested that the person that prepares Druid Release Notes (I
> >> think
> >> > it's Jon usually) goes through all PRs labelled "Area - Metrics/Event
> >> > Emitting" (
> >> >
> >> >
> >>
> https://github.com/apache/incubator-druid/pulls?q=is%3Apr+sort%3Aupdated-desc+label%3A%22Area+-+Metrics%2FEvent+Emitting%22+is%3Aclosed+milestone%3A0.13.0
> >> > )
> >> > along with "Release Notes", to present this information in the release
> >> > notes.
> >> >
> >> > BTW I wonder is the a document in the repository or elsewhere that
> >> > describes the release process?
> >> >
> >>
> >
>

Re: [VOTE] Release Apache Druid (incubating) 0.13.0 [RC1]

[Roman Leventov]

Another issue that needs to be resolved before the release is
https://github.com/apache/incubator-druid/issues/6559

On Wed, 31 Oct 2018 at 00:12, Julian Hyde  wrote:

>
>
> > On Oct 30, 2018, at 12:04 PM, Gian Merlino  wrote:
> >
> > It says
> > to use http://static.druid.io/, but we probably won't put the artifacts
> > there.
>
> Indeed. It is mandatory that the artifacts and their checksums are hosted
> on apache.org . (There can be mirrors.)
>
> Julian
>
>

Re: Metrics updates in Release Notes

[Roman Leventov]

I think because Metrics / Event Emitting PRs usually make something
that needs to be reflected in the release notes, while ordinary PRs - not
so often.

On Wed, 31 Oct 2018 at 18:47, Gian Merlino  wrote:

> Why is "metrics / event emitting" special, though? Why shouldn't we ask the
> release manager to look at _all_ tags just in case? (Which is clearly too
> much burden for a release manager -- I'm trying to make an argument, I
> guess, that it's fair to push some of the burden to the committer that
> originally merged the PR.)
>
> On Wed, Oct 31, 2018 at 10:44 AM Roman Leventov 
> wrote:
>
> > I think people will often forget to put both tags, so the person who does
> > the release should check the tag Metrics/Event Emitting anyway, just in
> > case.
> >
> > On Wed, 31 Oct 2018 at 18:09, Gian Merlino  wrote:
> >
> > > I don't think we have a doc about how to do a release, but yeah it
> would
> > be
> > > great to have it. Dave, would you be able to put it together while you
> > > manage this release? I am sure it will differ substantially from what
> > we've
> > > done in the past, because of the new Apache-ified stuff.
> > >
> > > On Wed, Oct 31, 2018 at 10:07 AM Gian Merlino  wrote:
> > >
> > > > Why not also tag those with "Release Notes"? It makes it a lot easier
> > for
> > > > release managers to do their jobs if they just have to look at one
> > label.
> > > > (Or two, I guess: "release notes" and "incompatible". But I would be
> > down
> > > > to merge them.)
> > > >
> > > > On Wed, Oct 31, 2018 at 9:26 AM David Lim 
> wrote:
> > > >
> > > >> Thanks Roman. I'm helping with the release this time so I will check
> > the
> > > >> PRs with that label and include them in the release notes as
> > > appropriate.
> > > >>
> > > >> As far as I know, there isn't any document like that, but I agree it
> > > would
> > > >> be quite useful.
> > > >>
> > > >> On Wed, Oct 31, 2018 at 9:04 AM Roman Leventov  >
> > > >> wrote:
> > > >>
> > > >> > It's suggested that the person that prepares Druid Release Notes
> (I
> > > >> think
> > > >> > it's Jon usually) goes through all PRs labelled "Area -
> > Metrics/Event
> > > >> > Emitting" (
> > > >> >
> > > >> >
> > > >>
> > >
> >
> https://github.com/apache/incubator-druid/pulls?q=is%3Apr+sort%3Aupdated-desc+label%3A%22Area+-+Metrics%2FEvent+Emitting%22+is%3Aclosed+milestone%3A0.13.0
> > > >> > )
> > > >> > along with "Release Notes", to present this information in the
> > release
> > > >> > notes.
> > > >> >
> > > >> > BTW I wonder is the a document in the repository or elsewhere that
> > > >> > describes the release process?
> > > >> >
> > > >>
> > > >
> > >
> >
>

SegmentId PR

[Roman Leventov]

Could somebody please provide design review of "Introduce SegmentId class"
PR : https://github.com/apache/incubator-druid/pull/6370? This is an
important improvement, and many other improvements and bugs fixes are
blocked on it. Despite "Development Blocker" tag (that was thought to give
PRs a priority), nobody reviewed this PR for almost two months, except Egor
with whom we work for the same company.

Re: SegmentId PR

[Roman Leventov]

Because in a lot of places, SegmentId is deserialized from "segment id
string", that doesn't have enough information to reconstruct the ShardSpec.

On Mon, 12 Nov 2018 at 17:28, Gian Merlino  wrote:

> I could take a look after 0.13.0 is released. Right now things related to
> that are the main things I am spending my Druid-related time on.
>
> I haven't read most of the diff yet, but I was wondering, is there a reason
> you make a new class instead of using SegmentIdentifier? They are slightly
> different (one has a ShardSpec and one just has the partition num) but I am
> wondering if these need to be two different classes or not.
>
> On Mon, Nov 12, 2018 at 4:51 AM Roman Leventov 
> wrote:
>
> > Could somebody please provide design review of "Introduce SegmentId
> class"
> > PR : https://github.com/apache/incubator-druid/pull/6370? This is an
> > important improvement, and many other improvements and bugs fixes are
> > blocked on it. Despite "Development Blocker" tag (that was thought to
> give
> > PRs a priority), nobody reviewed this PR for almost two months, except
> Egor
> > with whom we work for the same company.
> >
>

Druid PR review checklist

[Roman Leventov]

A lot of new committers are expected to enter the projects with rights to
review and merge PRs.

I suggest to create a PR review checklist to help new (and old!) reviewers
(and PR authors, for self-review before even publishing a PR) not to forget
something.

I think a PR (because it's not editable by many people) or a Wiki page
(because it's not commentable) on Github is not an ideal form of
collaboration for creating an original version of such document, so I
created a Google document (commentable, editable):
https://docs.google.com/document/d/17EEKT6fih9Dd5NfXjBoECcKbVp1eOB2vb3jKqTF9pPc/edit?usp=sharing

Developers are welcome to add comments and list things that they look at
when doing reviews.

Note: the list is going to be huge and people are not realistically
expected to pedantically follow all of it's items on every PR review, but
IMO such "gold standard" should help to keep the quality of reviews high.

Re: Druid PR review checklist

[Roman Leventov]

Yes, definitely, that is what I was planning to do. Except that I would say
that it could take closer to a month to complete a document.

On Tue, 13 Nov 2018, 18:56 Julian Hyde  Thanks for starting this thread, Roman. It’s a great discussion to be
> having.
>
> A word of caution about google docs. Since this one can be edited by
> anyone who has the link, and the link is posted in a public archive, then
> at some point this doc will fall victim to spam or vandalism. I suggest
> that after this discussion has died down (say a week or so?) you move the
> content to a more protected medium, say a GitHub PR, and remove the doc or
> make it read-only.
>
> Julian
>
>
> > On Nov 12, 2018, at 2:42 PM, Roman Leventov  wrote:
> >
> > A lot of new committers are expected to enter the projects with rights to
> > review and merge PRs.
> >
> > I suggest to create a PR review checklist to help new (and old!)
> reviewers
> > (and PR authors, for self-review before even publishing a PR) not to
> forget
> > something.
> >
> > I think a PR (because it's not editable by many people) or a Wiki page
> > (because it's not commentable) on Github is not an ideal form of
> > collaboration for creating an original version of such document, so I
> > created a Google document (commentable, editable):
> >
> https://docs.google.com/document/d/17EEKT6fih9Dd5NfXjBoECcKbVp1eOB2vb3jKqTF9pPc/edit?usp=sharing
> >
> > Developers are welcome to add comments and list things that they look at
> > when doing reviews.
> >
> > Note: the list is going to be huge and people are not realistically
> > expected to pedantically follow all of it's items on every PR review, but
> > IMO such "gold standard" should help to keep the quality of reviews high.
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> For additional commands, e-mail: dev-h...@druid.apache.org
>
>

Re: Druid PR review checklist

[Roman Leventov]

I don't think that would be convenient. I don't want to act as a gatekeeper
for every change and contribution to the doc.

On Tue, 13 Nov 2018, 19:34 Slim Bouguerra  Great Doc.
> Since the end goal is a .md document as part of the github repo.  How about
> you actually start a PR and with something like mark down document and
> peoples can interact with it via git comments, that will be better than
> google Docs thought.
>
> On Tue, Nov 13, 2018 at 10:25 AM Roman Leventov 
> wrote:
>
> > Yes, definitely, that is what I was planning to do. Except that I would
> say
> > that it could take closer to a month to complete a document.
> >
> > On Tue, 13 Nov 2018, 18:56 Julian Hyde  >
> > > Thanks for starting this thread, Roman. It’s a great discussion to be
> > > having.
> > >
> > > A word of caution about google docs. Since this one can be edited by
> > > anyone who has the link, and the link is posted in a public archive,
> then
> > > at some point this doc will fall victim to spam or vandalism. I suggest
> > > that after this discussion has died down (say a week or so?) you move
> the
> > > content to a more protected medium, say a GitHub PR, and remove the doc
> > or
> > > make it read-only.
> > >
> > > Julian
> > >
> > >
> > > > On Nov 12, 2018, at 2:42 PM, Roman Leventov 
> > wrote:
> > > >
> > > > A lot of new committers are expected to enter the projects with
> rights
> > to
> > > > review and merge PRs.
> > > >
> > > > I suggest to create a PR review checklist to help new (and old!)
> > > reviewers
> > > > (and PR authors, for self-review before even publishing a PR) not to
> > > forget
> > > > something.
> > > >
> > > > I think a PR (because it's not editable by many people) or a Wiki
> page
> > > > (because it's not commentable) on Github is not an ideal form of
> > > > collaboration for creating an original version of such document, so I
> > > > created a Google document (commentable, editable):
> > > >
> > >
> >
> https://docs.google.com/document/d/17EEKT6fih9Dd5NfXjBoECcKbVp1eOB2vb3jKqTF9pPc/edit?usp=sharing
> > > >
> > > > Developers are welcome to add comments and list things that they look
> > at
> > > > when doing reviews.
> > > >
> > > > Note: the list is going to be huge and people are not realistically
> > > > expected to pedantically follow all of it's items on every PR review,
> > but
> > > > IMO such "gold standard" should help to keep the quality of reviews
> > high.
> > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> > > For additional commands, e-mail: dev-h...@druid.apache.org
> > >
> > >
> >
>

PR Milestone policy

[Roman Leventov]

About a year ago, Gian wrote (
https://groups.google.com/forum/#!msg/druid-development/QPZUIzLtZ2I/eZyw8BoVCgAJ
):

"For milestones, I think it would work to use them only for PRs/issues that
are truly release blockers -- should be limited to critical bugs discovered
after a release branch is cut. We could make release notes the way you
suggest, by walking the commit history."

Today, Fangjin wrote (
https://github.com/apache/incubator-druid/pull/6656#issuecomment-441698159):

"I think where possible we should try to assign milestones to PRs we want
to get in and aim to have the PR reviewed and merged before then. If the PR
needs to be pushed back to a future release we can always do that."

Personally I don't agree with the second take and differentiating non-bug
fixing PRs by their "importance". I think the proportion of PRs that are
assigned the next milestone by committer will depend on self-confidence of
the committer and politics, not the objective importance of the PRs. It
will also make possible for some minor PRs to be sidetracked for extremely
long time if not forever, because there always other more important PRs to
work on. While true in the short and mid run, this is very frustrating for
the authors and could turn them away from contributing into Druid, that is
bad in the long run. Actually this thing happens already sometimes and we
should think how to address that, but differentiating PRs could
only exacerbate this effect.

Instead, I think the importance of PR should grow with the time passed
since the author addressed all comments (or just created the PR) and the PR
passed automated checks. I. e. a freshly created PR may be not super
important, but if it passes all checks and is open for two months without
reviews, the PR becomes more important to review. This should help to
reduce the variance in PR's time-to-merge and improve the average
contributor experience. In the long run I think it's healthier than
squeezing one extra feature into the very next release.

Sources of Reference objects in Druid

[Roman Leventov]

The discussion has started here:
https://github.com/apache/incubator-druid/pull/6677#discussion_r237182258
(TL;DR: there are a lot of references from java.lang.ref package, including
finalizers, in the heaps of Druid nodes, that is bad for GC).

Now I'm 90% sure that the major source of finalizable objects is Curator:
https://issues.apache.org/jira/browse/CURATOR-487

We are not exposed to that, but those who use Zstd compression with Druid
could have a lot of finalizable objects coming from Zstd compression, see
https://issues.apache.org/jira/browse/COMPRESS-473 and
https://github.com/luben/zstd-jni/issues/83

Zstd compression / decompression could also be a subject of double-free
race through finalize(): https://github.com/luben/zstd-jni/issues/82

I didn't check other compression/decompression/encryption/decryption parts
used by Druid yet but there is a big chance that some of them are
problematic too.

Resume making timely releases

[Roman Leventov]

I suggest resuming making quarterly releases. 0.13 branch was created
between October 12 and 19, as far as I can see. Then 0.14 branch should be
created between 12 and 19 of January.

Re: PR Milestone policy

[Roman Leventov]

Fangjin, what you suggest will lead to just one thing - all committers will
always assign their PRs to the next release milestone. In addition, you
also assign PRs from non-committers to the next release milestone. So
nearly 100% of new PRs will have that milestone. It will make this whole
activity pointless, because the milestone will not tell release managers
anything. Except maybe creating unneeded sense of rush.

Currently in Druid, there is a good fraction of PRs that are merged very
quickly (in a matter of days and sometimes hours), but there are also quite
some less lucky PRs that linger for months. For contributors, it's not very
important that the PR is merged in 1 hour, it's more important that it
appears in the next release. Therefore we need to optimize for the fraction
of PRs that are merged in 1 month or less (the average time between
creation of a new release branch and a final release date). Reviewers
should schedule their time so that there are less PRs that are merged in
less than one day, but more PRs that are merged in less than one month.

On Tue, 4 Dec 2018 at 04:28, Julian Hyde  wrote:

> I agree with you that merging PRs promptly is very important for growing
> community. Or, if the PR is inadequate, promptly explain to the contributor
> what they can do to improve it.
>
> Assigning target milestones to bugs and issues that don’t yet have PRs can
> be problematic. The person assigning the milestone has stepped into the
> role of project manager, unless they are committing to fix the issue
> personally. And even then, they are implicitly saying “hold the release
> while I work on this code”, which should really be the responsibility of
> the release manager alone.
>
> Julian
>
>
>
>
> > On Dec 3, 2018, at 1:57 PM, Fangjin Yang  wrote:
> >
> > Committers. In general I think we should try to be more inclusive of the
> > community and people that are interested in contributing to Druid and try
> > to get their PRs in as much as possible. This helps to grow the
> community.
> > To me, this means assigning milestones to contributions, not being overly
> > picky on code (if it has no real impact on functionality/performance). If
> > we need to push PRs back to a later release because they are complicated
> > and require more review, we can always do that.
> >
> > On Tue, Nov 27, 2018 at 4:45 PM Julian Hyde  wrote:
> >
> >> Fangjin,
> >>
> >> You wrote
> >>
> >>> we should try to assign milestones to PRs we want
> >>> to get in
> >>
> >> Can you please define “we”? Do you mean committers, PMC members, release
> >> managers, everyone?
> >>
> >> Julian
> >>
> >>
> >>> On Nov 26, 2018, at 8:43 AM, Roman Leventov 
> wrote:
> >>>
> >>> About a year ago, Gian wrote (
> >>>
> >>
> https://groups.google.com/forum/#!msg/druid-development/QPZUIzLtZ2I/eZyw8BoVCgAJ
> >>> ):
> >>>
> >>> "For milestones, I think it would work to use them only for PRs/issues
> >> that
> >>> are truly release blockers -- should be limited to critical bugs
> >> discovered
> >>> after a release branch is cut. We could make release notes the way you
> >>> suggest, by walking the commit history."
> >>>
> >>> Today, Fangjin wrote (
> >>>
> >>
> https://github.com/apache/incubator-druid/pull/6656#issuecomment-441698159
> >> ):
> >>>
> >>> "I think where possible we should try to assign milestones to PRs we
> want
> >>> to get in and aim to have the PR reviewed and merged before then. If
> the
> >> PR
> >>> needs to be pushed back to a future release we can always do that."
> >>>
> >>> Personally I don't agree with the second take and differentiating
> non-bug
> >>> fixing PRs by their "importance". I think the proportion of PRs that
> are
> >>> assigned the next milestone by committer will depend on self-confidence
> >> of
> >>> the committer and politics, not the objective importance of the PRs. It
> >>> will also make possible for some minor PRs to be sidetracked for
> >> extremely
> >>> long time if not forever, because there always other more important PRs
> >> to
> >>> work on. While true in the short and mid run, this is very frustrating
> >> for
> >>> the authors and could turn them away from contributing into Druid, that
> >> is
> >>> bad in the long run. Actually this thing happens already 

Re: PR Milestone policy

[Roman Leventov]

The previous consensus community decision seems to be to not use PR
milestones for any PRs except bugs. To change this policy, probably there
should be a committer (or PPMC?) vote.

On Thu, 6 Dec 2018 at 20:49, Julian Hyde  wrote:

> FJ,
>
> What you are proposing sounds suspiciously like project management. If a
> contributor makes a contribution, that contribution should be given a fair
> review in a timely fashion and be committed based on its merits. You
> overstate the time-sensitivity of contributions. I would imagine that there
> are only a few days preceding each release where stability is a major
> concern. At any other times, contributions can go in after a review.
>
> Remember that in Apache, no one person or group of people determines the
> technical direction of the project, nor the timing of the releases.
> Contributions are accepted based on merit, and release timing is determined
> by consensus.
>
> Let’s be sure not to overuse milestone policy. Milestones should be for
> guidance only.
>
> Julian
>
>
> > On Dec 6, 2018, at 10:12 AM, Fangjin Yang  wrote:
> >
> > Roman - one of the roles of a committer is to make decisions on what is
> > best for Druid and the Druid community. If a committer feels that their
> PR
> > should be included in the next release, they should make an argument of
> why
> > that is. Conversely, if folks in the community feel that a PR should not
> be
> > included, they should be free to voice their opinion as well.
> >
> > Many of the community contributions I see today are adding value to the
> > project and we should try to include them in upcoming releases. The PRs I
> > see adding no value are unnecessary refactoring of that serve no real
> > purpose. They don't make the code stable, easier to maintain, or add new
> > features, and look to be submitted only to increase total contribution
> line
> > count to Druid. I think we should aim to prevent these types of PRs in
> any
> > release because they don't serve to benefit the community.
> >
> > On Tue, Dec 4, 2018 at 5:24 AM Roman Leventov 
> wrote:
> >
> >> Fangjin, what you suggest will lead to just one thing - all committers
> will
> >> always assign their PRs to the next release milestone. In addition, you
> >> also assign PRs from non-committers to the next release milestone. So
> >> nearly 100% of new PRs will have that milestone. It will make this whole
> >> activity pointless, because the milestone will not tell release managers
> >> anything. Except maybe creating unneeded sense of rush.
> >>
> >> Currently in Druid, there is a good fraction of PRs that are merged very
> >> quickly (in a matter of days and sometimes hours), but there are also
> quite
> >> some less lucky PRs that linger for months. For contributors, it's not
> very
> >> important that the PR is merged in 1 hour, it's more important that it
> >> appears in the next release. Therefore we need to optimize for the
> fraction
> >> of PRs that are merged in 1 month or less (the average time between
> >> creation of a new release branch and a final release date). Reviewers
> >> should schedule their time so that there are less PRs that are merged in
> >> less than one day, but more PRs that are merged in less than one month.
> >>
> >> On Tue, 4 Dec 2018 at 04:28, Julian Hyde  wrote:
> >>
> >>> I agree with you that merging PRs promptly is very important for
> growing
> >>> community. Or, if the PR is inadequate, promptly explain to the
> >> contributor
> >>> what they can do to improve it.
> >>>
> >>> Assigning target milestones to bugs and issues that don’t yet have PRs
> >> can
> >>> be problematic. The person assigning the milestone has stepped into the
> >>> role of project manager, unless they are committing to fix the issue
> >>> personally. And even then, they are implicitly saying “hold the release
> >>> while I work on this code”, which should really be the responsibility
> of
> >>> the release manager alone.
> >>>
> >>> Julian
> >>>
> >>>
> >>>
> >>>
> >>>> On Dec 3, 2018, at 1:57 PM, Fangjin Yang  wrote:
> >>>>
> >>>> Committers. In general I think we should try to be more inclusive of
> >> the
> >>>> community and people that are interested in contributing to Druid and
> >> try
> >>>> to get their PRs in as much as possible. 

Re: PR Milestone policy

[Roman Leventov]

I would like like learn what is the Apache way to resolve debates. But you
are right, this question probably doesn't deserve that. Thanks for guidance
Julian.

On Fri, 7 Dec 2018 at 16:43, Julian Hyde  wrote:

> May I suggest that a vote is not the solution. In this discussion I see
> two people beating each other over the head with policy.
>
> Let’s strive to operate according to the Apache way. Accept contributions
> on merit in a timely manner. Avoid the urge to “project manage”.
>
> Julian
>
> > On Dec 7, 2018, at 07:03, Roman Leventov  wrote:
> >
> > The previous consensus community decision seems to be to not use PR
> > milestones for any PRs except bugs. To change this policy, probably there
> > should be a committer (or PPMC?) vote.
> >
> >> On Thu, 6 Dec 2018 at 20:49, Julian Hyde  wrote:
> >>
> >> FJ,
> >>
> >> What you are proposing sounds suspiciously like project management. If a
> >> contributor makes a contribution, that contribution should be given a
> fair
> >> review in a timely fashion and be committed based on its merits. You
> >> overstate the time-sensitivity of contributions. I would imagine that
> there
> >> are only a few days preceding each release where stability is a major
> >> concern. At any other times, contributions can go in after a review.
> >>
> >> Remember that in Apache, no one person or group of people determines the
> >> technical direction of the project, nor the timing of the releases.
> >> Contributions are accepted based on merit, and release timing is
> determined
> >> by consensus.
> >>
> >> Let’s be sure not to overuse milestone policy. Milestones should be for
> >> guidance only.
> >>
> >> Julian
> >>
> >>
> >>> On Dec 6, 2018, at 10:12 AM, Fangjin Yang  wrote:
> >>>
> >>> Roman - one of the roles of a committer is to make decisions on what is
> >>> best for Druid and the Druid community. If a committer feels that their
> >> PR
> >>> should be included in the next release, they should make an argument of
> >> why
> >>> that is. Conversely, if folks in the community feel that a PR should
> not
> >> be
> >>> included, they should be free to voice their opinion as well.
> >>>
> >>> Many of the community contributions I see today are adding value to the
> >>> project and we should try to include them in upcoming releases. The
> PRs I
> >>> see adding no value are unnecessary refactoring of that serve no real
> >>> purpose. They don't make the code stable, easier to maintain, or add
> new
> >>> features, and look to be submitted only to increase total contribution
> >> line
> >>> count to Druid. I think we should aim to prevent these types of PRs in
> >> any
> >>> release because they don't serve to benefit the community.
> >>>
> >>> On Tue, Dec 4, 2018 at 5:24 AM Roman Leventov 
> >> wrote:
> >>>
> >>>> Fangjin, what you suggest will lead to just one thing - all committers
> >> will
> >>>> always assign their PRs to the next release milestone. In addition,
> you
> >>>> also assign PRs from non-committers to the next release milestone. So
> >>>> nearly 100% of new PRs will have that milestone. It will make this
> whole
> >>>> activity pointless, because the milestone will not tell release
> managers
> >>>> anything. Except maybe creating unneeded sense of rush.
> >>>>
> >>>> Currently in Druid, there is a good fraction of PRs that are merged
> very
> >>>> quickly (in a matter of days and sometimes hours), but there are also
> >> quite
> >>>> some less lucky PRs that linger for months. For contributors, it's not
> >> very
> >>>> important that the PR is merged in 1 hour, it's more important that it
> >>>> appears in the next release. Therefore we need to optimize for the
> >> fraction
> >>>> of PRs that are merged in 1 month or less (the average time between
> >>>> creation of a new release branch and a final release date). Reviewers
> >>>> should schedule their time so that there are less PRs that are merged
> in
> >>>> less than one day, but more PRs that are merged in less than one
> month.
> >>>>
> >>>>> On Tue, 4 Dec 2018 at 04:28, Julian Hyde  wrote:
> >>>>&

Re: PR Milestone policy

[Roman Leventov]

It's not exactly and not only that. I advocate for not assigning milestones
to any non-bug (or otherwise "critical") PRs, including "feature",
non-refactoring PRs.

On Fri, 7 Dec 2018 at 19:29, Julian Hyde  wrote:

> Consensus.
>
> We resolve debates by going into them knowing that we need to find
> consensus. A vote is a last step to prove that consensus exists, and
> in most cases is not necessary.
>
> Reading between the lines, it sounds as if you and FJ have a
> difference of opinion about refactoring changes. Two extreme positions
> would be (1) we don't accept changes that only refactor code, (2) and
> I assert my right to contribute a refactoring change at any point in
> the project lifecycle. A debate that starts with those positions is
> never going to reach consensus. A better starting point might be "I
> would like to make the following change because I believe it would be
> beneficial. How could I best structure it / time it to minimize
> impact?"
> On Fri, Dec 7, 2018 at 9:19 AM Roman Leventov 
> wrote:
> >
> > I would like like learn what is the Apache way to resolve debates. But
> you
> > are right, this question probably doesn't deserve that. Thanks for
> guidance
> > Julian.
> >
> > On Fri, 7 Dec 2018 at 16:43, Julian Hyde  wrote:
> >
> > > May I suggest that a vote is not the solution. In this discussion I see
> > > two people beating each other over the head with policy.
> > >
> > > Let’s strive to operate according to the Apache way. Accept
> contributions
> > > on merit in a timely manner. Avoid the urge to “project manage”.
> > >
> > > Julian
> > >
> > > > On Dec 7, 2018, at 07:03, Roman Leventov 
> wrote:
> > > >
> > > > The previous consensus community decision seems to be to not use PR
> > > > milestones for any PRs except bugs. To change this policy, probably
> there
> > > > should be a committer (or PPMC?) vote.
> > > >
> > > >> On Thu, 6 Dec 2018 at 20:49, Julian Hyde  wrote:
> > > >>
> > > >> FJ,
> > > >>
> > > >> What you are proposing sounds suspiciously like project management.
> If a
> > > >> contributor makes a contribution, that contribution should be given
> a
> > > fair
> > > >> review in a timely fashion and be committed based on its merits. You
> > > >> overstate the time-sensitivity of contributions. I would imagine
> that
> > > there
> > > >> are only a few days preceding each release where stability is a
> major
> > > >> concern. At any other times, contributions can go in after a review.
> > > >>
> > > >> Remember that in Apache, no one person or group of people
> determines the
> > > >> technical direction of the project, nor the timing of the releases.
> > > >> Contributions are accepted based on merit, and release timing is
> > > determined
> > > >> by consensus.
> > > >>
> > > >> Let’s be sure not to overuse milestone policy. Milestones should be
> for
> > > >> guidance only.
> > > >>
> > > >> Julian
> > > >>
> > > >>
> > > >>> On Dec 6, 2018, at 10:12 AM, Fangjin Yang 
> wrote:
> > > >>>
> > > >>> Roman - one of the roles of a committer is to make decisions on
> what is
> > > >>> best for Druid and the Druid community. If a committer feels that
> their
> > > >> PR
> > > >>> should be included in the next release, they should make an
> argument of
> > > >> why
> > > >>> that is. Conversely, if folks in the community feel that a PR
> should
> > > not
> > > >> be
> > > >>> included, they should be free to voice their opinion as well.
> > > >>>
> > > >>> Many of the community contributions I see today are adding value
> to the
> > > >>> project and we should try to include them in upcoming releases. The
> > > PRs I
> > > >>> see adding no value are unnecessary refactoring of that serve no
> real
> > > >>> purpose. They don't make the code stable, easier to maintain, or
> add
> > > new
> > > >>> features, and look to be submitted only to increase total
> contribution
> > > >> line
> > > >>> count to Druid. I think we should aim to prevent

Re: SegmentId PR

[Roman Leventov]

It's now almost three months since this PR was opened. Could any committer
(non necessarily Gian) please provide a design review?

I want to remind that in PRs labelled "Design review", the second review is
not required to be full, a line-by-line textual review, it could literally
be just "design review". So in the context of the SegmentId PR, which Egor
has already reviewed in full, it should not be as time-consuming as
reviewing some other +3700/-3700 line PR. A big portion of that are
mechanical API changes across the project, in tests and benchmarks. As I
noted here:
https://github.com/apache/incubator-druid/pull/6370#issuecomment-423731847,
there are only 6 classes that are added or changed meaningfully.

On Mon, 12 Nov 2018 at 21:24, Roman Leventov  wrote:

> Because in a lot of places, SegmentId is deserialized from "segment id
> string", that doesn't have enough information to reconstruct the ShardSpec.
>
> On Mon, 12 Nov 2018 at 17:28, Gian Merlino  wrote:
>
>> I could take a look after 0.13.0 is released. Right now things related to
>> that are the main things I am spending my Druid-related time on.
>>
>> I haven't read most of the diff yet, but I was wondering, is there a
>> reason
>> you make a new class instead of using SegmentIdentifier? They are slightly
>> different (one has a ShardSpec and one just has the partition num) but I
>> am
>> wondering if these need to be two different classes or not.
>>
>> On Mon, Nov 12, 2018 at 4:51 AM Roman Leventov 
>> wrote:
>>
>> > Could somebody please provide design review of "Introduce SegmentId
>> class"
>> > PR : https://github.com/apache/incubator-druid/pull/6370? This is an
>> > important improvement, and many other improvements and bugs fixes are
>> > blocked on it. Despite "Development Blocker" tag (that was thought to
>> give
>> > PRs a priority), nobody reviewed this PR for almost two months, except
>> Egor
>> > with whom we work for the same company.
>> >
>>
>

Drop 0. from the version

[Roman Leventov]

It doesn't seem to me that Druid API is going to stabilize in the near
future (if ever), because there are so many extension points and something
is broken in every release. On the other hand, Druid is not Hadoop or
Spark, which have applications API. Druid API for extensions, not
applications. It is used by people who are closer to Druid development and
fixing their extensions is routine.

With that, I think it make sense to drop "0." from the Druid version and
call it Druid 14, Druid 15, etc.

Re: Drop 0. from the version

[Roman Leventov]

It may also make sense to distinguish "operations" breaking changes from
API breaking changes. Operations breaking changes establish the minimum
cadence of Druid cluster upgrades, that allow rolling Druid versions back
and forward. I. e. it's related to segment format, the format of the data
kept in ZooKeeper and the SQL database, or events such as stopping support
of ZooKeeper for certain things (e. g. forcing using of HTTP
announcements). So Druid cluster operators cannot update Druid from version
X to version Z skipping the version Y, if both Y and Z have some operations
breaking changes. (Any such changes should support rollback options at
least until the next version with operations breaking changes.)

API breaking changes are just changes in Druid extensions APIs. Druid
cluster operators could skip any number of releases with such breaking
changes, as long as their extension's code is updated for the latest
version of API.

On Thu, 20 Dec 2018 at 20:03, Roman Leventov  wrote:

> It doesn't seem to me that Druid API is going to stabilize in the near
> future (if ever), because there are so many extension points and something
> is broken in every release. On the other hand, Druid is not Hadoop or
> Spark, which have applications API. Druid API for extensions, not
> applications. It is used by people who are closer to Druid development and
> fixing their extensions is routine.
>
> With that, I think it make sense to drop "0." from the Druid version and
> call it Druid 14, Druid 15, etc.
>

Re: Drop 0. from the version

[Roman Leventov]

> >
> > > > If this sounds reasonable then jumping straight to Apache Druid 14 on
> > the
> > > > first official apache release would make a lot of sense.
> > > >
> > > > Cheers,
> > > > Charles Allen
> > > >
> > > >
> > > > On Thu, Dec 20, 2018 at 11:07 PM Gian Merlino 
> wrote:
> > > >
> > > > > I think it's a good point. Culturally we have been willing to break
> > > > > extension APIs for relatively small benefits. But we have generally
> > > been
> > > > > unwilling to make breaking changes on the operations side quite so
> > > > > liberally. Also, most cluster operators don't have their own custom
> > > > > extensions, in my experience. So it does make sense to
> differentiate
> > > > them.
> > > > > I'm not sure how it makes sense to differentiate them, though. It
> > could
> > > > be
> > > > > done through the version number (only increment the major version
> for
> > > > > operations breaking changes) or it could be done through an
> > "upgrading"
> > > > > guide in the documentation (increment the major version for
> > operations
> > > or
> > > > > extension breaking changes, but, have a guide that tells people
> which
> > > > > versions have operations breaking changes to aid in upgrades).
> > > > >
> > > > > Coming back to the question in the subject of your mail: IMO, for
> > > > > "graduation" out of 0.x, we should talk as a community about what
> > that
> > > > > means to us. It is a milestone that on the one hand, doesn't mean
> > much,
> > > > but
> > > > > on the other hand, can be deeply symbolic. Some things that it has
> > > meant
> > > > to
> > > > > other projects:
> > > > >
> > > > > 1) Production readiness. Obviously Druid is well past this. If this
> > is
> > > > what
> > > > > dropping the 0. means, then we should do it immediately.
> > > > >
> > > > > 2) Belief that the APIs have become relatively stable. Like you
> said,
> > > the
> > > > > extension APIs don't seem particularly close to stable, but maybe
> > > that's
> > > > > okay. However, the pace of breaking changes on the operations and
> > query
> > > > > side for non-experimental features has been relatively calm for the
> > > past
> > > > > couple of years, so if we focus on that then we can make a case
> here.
> > > > >
> > > > > 3) Completeness of vision. This one is the most interesting to me.
> I
> > > > > suspect that different people in the community have different
> visions
> > > for
> > > > > Druid. It is also the kind of project that may never truly be
> > complete
> > > in
> > > > > vision (in principle, the platform could become a competitive data
> > > > > warehouse, search engine, etc, …). For what it's worth, my vision
> of
> > > > Druid
> > > > > for the next year at least involves robust stream ingestion being a
> > > first
> > > > > class ingestion method (Kafka / Kinesis indexing service style) and
> > SQL
> > > > > being a first class query language. These are both, today, still
> > > > > experimental features. So are lookups. All of these 3 features,
> from
> > > > what I
> > > > > can see, are quite popular amongst Druid users despite being
> > > > experimental.
> > > > > For a 'completeness of vision' based 1.0 I would want to lift all
> of
> > > > those
> > > > > out of experimental status and, for SQL in particular, to have its
> > > > > functionality rounded out a bit more (to support the native query
> > > > features
> > > > > it doesn't currently support, like multi-value dimensions,
> > > datasketches,
> > > > > etc).
> > > > >
> > > > > 4) Marketing / timing. Like, doing a 1.0 around the time we
> graduate
> > > from
> > > > > the Incubator. Not sure how much this really matters, but projects
> do
> > > it
> > > > > sometimes.
> > > > >
> > > > > Another question is, how often do we inten

Re: Drop 0. from the version

[Roman Leventov]

his
> > > is
> > > > > what
> > > > > > dropping the 0. means, then we should do it immediately.
> > > > > >
> > > > > > 2) Belief that the APIs have become relatively stable. Like you
> said,
> > > > the
> > > > > > extension APIs don't seem particularly close to stable, but maybe
> > > > that's
> > > > > > okay. However, the pace of breaking changes on the operations and
> > > query
> > > > > > side for non-experimental features has been relatively calm for
> the
> > > > past
> > > > > > couple of years, so if we focus on that then we can make a case
> here.
> > > > > >
> > > > > > 3) Completeness of vision. This one is the most interesting to
> me. I
> > > > > > suspect that different people in the community have different
> visions
> > > > for
> > > > > > Druid. It is also the kind of project that may never truly be
> > > complete
> > > > in
> > > > > > vision (in principle, the platform could become a competitive
> data
> > > > > > warehouse, search engine, etc, …). For what it's worth, my
> vision of
> > > > > Druid
> > > > > > for the next year at least involves robust stream ingestion
> being a
> > > > first
> > > > > > class ingestion method (Kafka / Kinesis indexing service style)
> and
> > > SQL
> > > > > > being a first class query language. These are both, today, still
> > > > > > experimental features. So are lookups. All of these 3 features,
> from
> > > > > what I
> > > > > > can see, are quite popular amongst Druid users despite being
> > > > > experimental.
> > > > > > For a 'completeness of vision' based 1.0 I would want to lift
> all of
> > > > > those
> > > > > > out of experimental status and, for SQL in particular, to have
> its
> > > > > > functionality rounded out a bit more (to support the native query
> > > > > features
> > > > > > it doesn't currently support, like multi-value dimensions,
> > > > datasketches,
> > > > > > etc).
> > > > > >
> > > > > > 4) Marketing / timing. Like, doing a 1.0 around the time we
> graduate
> > > > from
> > > > > > the Incubator. Not sure how much this really matters, but
> projects do
> > > > it
> > > > > > sometimes.
> > > > > >
> > > > > > Another question is, how often do we intend to rev the version?
> At
> > > the
> > > > > rate
> > > > > > we're going, we rev 2-3 major versions a year. Would we intend to
> > > keep
> > > > > that
> > > > > > up, or slow it down by making more of an effort to avoid breaking
> > > > > changes?
> > > > > >
> > > > > > On Thu, Dec 20, 2018 at 2:17 PM Roman Leventov <
> > > leventov...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > It may also make sense to distinguish "operations" breaking
> changes
> > > > > from
> > > > > > > API breaking changes. Operations breaking changes establish the
> > > > minimum
> > > > > > > cadence of Druid cluster upgrades, that allow rolling Druid
> > > versions
> > > > > back
> > > > > > > and forward. I. e. it's related to segment format, the format
> of
> > > the
> > > > > data
> > > > > > > kept in ZooKeeper and the SQL database, or events such as
> stopping
> > > > > > support
> > > > > > > of ZooKeeper for certain things (e. g. forcing using of HTTP
> > > > > > > announcements). So Druid cluster operators cannot update Druid
> from
> > > > > > version
> > > > > > > X to version Z skipping the version Y, if both Y and Z have
> some
> > > > > > operations
> > > > > > > breaking changes. (Any such changes should support rollback
> options
> > > > at
> > > > > > > least until the next version with operations breaking changes.)
> > > > > > >
> > > > > > > API breaking changes are just changes in Druid extensions APIs.
> > > Druid
> > > > > > > cluster operators could skip any number of releases with such
> > > > breaking
> > > > > > > changes, as long as their extension's code is updated for the
> > > latest
> > > > > > > version of API.
> > > > > > >
> > > > > > > On Thu, 20 Dec 2018 at 20:03, Roman Leventov <
> leven...@apache.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > > It doesn't seem to me that Druid API is going to stabilize
> in the
> > > > > near
> > > > > > > > future (if ever), because there are so many extension points
> and
> > > > > > > something
> > > > > > > > is broken in every release. On the other hand, Druid is not
> > > Hadoop
> > > > or
> > > > > > > > Spark, which have applications API. Druid API for
> extensions, not
> > > > > > > > applications. It is used by people who are closer to Druid
> > > > > development
> > > > > > > and
> > > > > > > > fixing their extensions is routine.
> > > > > > > >
> > > > > > > > With that, I think it make sense to drop "0." from the Druid
> > > > version
> > > > > > and
> > > > > > > > call it Druid 14, Druid 15, etc.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> For additional commands, e-mail: dev-h...@druid.apache.org
>
>

Re: Off list major development

[Roman Leventov]

I see two important reasons why it makes sense to file an issue and
probably announce it in the mailing list, before writing a lot of code,
despite not having a clear picture of what it will be and any performance
data:
 1) somebody could already work on this problem privately in parallel, it
allows to avoid clash in people's works
 2) some people could quickly think about the problem field and share
high-level ideas that could wildly change the direction in which the author
(the person who is going to write code) will move in his work from early on.

Jihoon in
https://lists.apache.org/thread.html/e007fbf362c2a870a2d88d04431789289807e00fd91d087559a01d1f@%3Cdev.druid.apache.org%3E
and
later Gian in this thread suggested that _every_ piece of work that should
be labelled as "Design Review" according to the current rules should be
accompanied by an issue. I don't agree with this, there are some PRs as
small as a few dozens of lines of code, that add some configuration
parameter and therefore should be labelled "Design Review". I don't thing a
separate proposal issue is needed for them, and even for a little larger
PRs too.

For the same reason, I also don't see the point of renaming "Design Review"
into "Proposal", as well as separating "Design Review" into "Proposal" and
something like "API Review". I think a single "Design Review" tag handles
it well.

Gian mentioned an idea that PRs that follow a "Design Review" proposal
issue shouldn't be "Design Review" themselves. I don't agree with this, I
think that actual code and performance data are important inputs that
should be re-evaluated at least by two people. I even think that it's very
desirable that at least two people read _each line of production code_ in
large PRs, although it's not what was done historically in Druid, because
large bodies of newly added code, with whole new classes and subsystems
added, are also coincidentally tested worse than already existing classes
and subsystems, including in production. It seems to me that those huge
code influxes is a major source of bugs, that could later take years to
squeeze out from the codebase.

On Wed, 9 Jan 2019 at 08:24, Clint Wylie  wrote:

> Apologies for the delayed response.
>
> On Thu, Jan 3, 2019 at 12:48 PM Slim Bouguerra 
> wrote:
>
> > I am wondering here what is the case where code first is better?
> >
>
> I don't think it's wrong to share ideas as early as possible, and after
> this discussion I think I am in favor of it too. I just meant that I don't
> think it's always necessarily the most productive discussion until code
> exists sometimes, with the types of thing I am thinking of are almost
> entirely limited to cases where things might sound good to anyone on paper
> but in reality need a large amount of experiments conducted and
> observations collected to determine that something is actually worth doing,
> which I imagine is mainly things like reworking internals for performance
> improvements.
>
> In the case of my combined proposal PR, I needed to prove that the thing I
> was working on was a good idea... and it wasn't directly. But I came up
> with another idea during the course of experiment turned into something
> compelling, so an initial proposal would have looked quite a lot different
> than what I ended up with. Once I had proven to myself that it was a good
> idea, then I was comfortable sharing with the wider community. I'm not
> certain how this would play out in an always proposal first model, maybe
> the first proposal exists, I personally reject it after updating with
> experiment results show it's a bad idea, continue experimenting and raise a
> new one after the experiments start looking promising?
>
>
> > Let's not be naive this is very rare that a contributor will accept that
> > his work is to be thrown, usually devs takes coding as personal creation
> > and they get attached to it.
> >
>
> I agree, just because a handful of the committers have this attitude, it
> isn't fair to expect the wider community to also, that's why I am in favor
> of formalizing the process.
>
> Can you please explain what is overbearing ? what can be changed to make it
> > easy ?
> > Most of the points are kind of the actual questions that you want to
> > address before hand anyway isn't ?
> >
>
> Sorry for the confusion, I said it's "not overbearing", I think it's fine.
>
> What are the complaints ?
>
>
> Is this and other previous threads not a complaint about opening a large PR
> without a proposal? :) I just mean that formalizing the process, even if a
> proposal has a reference PR opened with it near concurrently, could prevent
> these discussions from happening in the future because ground rules have
> been set and we are all on the same page I guess.
>
> I think we should also probably consider renaming the "design review" label
> to "proposal" or something to make it more clear that a PR is associated
> with a proposal. It might also be worth considering using github projects
> fo

Re: Off list major development

[Roman Leventov]

What do community members think about also making a requirement that
"Design Review" PRs and proposals are reviewed by at least two people with
different affiliation? IMO it's a good idea, because it ensures that
different interests are taken into account. Also it pushes people to engage
with work done in other parts of Druid, improving shared code owning and
awareness.

Or it's against the Apache Way because it's assumed that there are no
company boundaries within the community?

On Thu, 10 Jan 2019 at 11:45, Roman Leventov  wrote:

> I see two important reasons why it makes sense to file an issue and
> probably announce it in the mailing list, before writing a lot of code,
> despite not having a clear picture of what it will be and any performance
> data:
>  1) somebody could already work on this problem privately in parallel, it
> allows to avoid clash in people's works
>  2) some people could quickly think about the problem field and share
> high-level ideas that could wildly change the direction in which the author
> (the person who is going to write code) will move in his work from early on.
>
> Jihoon in
> https://lists.apache.org/thread.html/e007fbf362c2a870a2d88d04431789289807e00fd91d087559a01d1f@%3Cdev.druid.apache.org%3E
>  and
> later Gian in this thread suggested that _every_ piece of work that should
> be labelled as "Design Review" according to the current rules should be
> accompanied by an issue. I don't agree with this, there are some PRs as
> small as a few dozens of lines of code, that add some configuration
> parameter and therefore should be labelled "Design Review". I don't thing a
> separate proposal issue is needed for them, and even for a little larger
> PRs too.
>
> For the same reason, I also don't see the point of renaming "Design
> Review" into "Proposal", as well as separating "Design Review" into
> "Proposal" and something like "API Review". I think a single "Design
> Review" tag handles it well.
>
> Gian mentioned an idea that PRs that follow a "Design Review" proposal
> issue shouldn't be "Design Review" themselves. I don't agree with this, I
> think that actual code and performance data are important inputs that
> should be re-evaluated at least by two people. I even think that it's very
> desirable that at least two people read _each line of production code_ in
> large PRs, although it's not what was done historically in Druid, because
> large bodies of newly added code, with whole new classes and subsystems
> added, are also coincidentally tested worse than already existing classes
> and subsystems, including in production. It seems to me that those huge
> code influxes is a major source of bugs, that could later take years to
> squeeze out from the codebase.
>
> On Wed, 9 Jan 2019 at 08:24, Clint Wylie  wrote:
>
>> Apologies for the delayed response.
>>
>> On Thu, Jan 3, 2019 at 12:48 PM Slim Bouguerra 
>> wrote:
>>
>> > I am wondering here what is the case where code first is better?
>> >
>>
>> I don't think it's wrong to share ideas as early as possible, and after
>> this discussion I think I am in favor of it too. I just meant that I don't
>> think it's always necessarily the most productive discussion until code
>> exists sometimes, with the types of thing I am thinking of are almost
>> entirely limited to cases where things might sound good to anyone on paper
>> but in reality need a large amount of experiments conducted and
>> observations collected to determine that something is actually worth
>> doing,
>> which I imagine is mainly things like reworking internals for performance
>> improvements.
>>
>> In the case of my combined proposal PR, I needed to prove that the thing I
>> was working on was a good idea... and it wasn't directly. But I came up
>> with another idea during the course of experiment turned into something
>> compelling, so an initial proposal would have looked quite a lot different
>> than what I ended up with. Once I had proven to myself that it was a good
>> idea, then I was comfortable sharing with the wider community. I'm not
>> certain how this would play out in an always proposal first model, maybe
>> the first proposal exists, I personally reject it after updating with
>> experiment results show it's a bad idea, continue experimenting and raise
>> a
>> new one after the experiments start looking promising?
>>
>>
>> > Let's not be naive this is very rare that a contributor will accept that
>> > his work is 

Re: Off list major development

[Roman Leventov]

It's not that people from one org could abuse the project and push some
change, but that they have similar perspective (bubble effect) and some
important aspects of a large feature could escape their attention.

I suggest it to be not a rigid rule, but a recommendation for authors of
large proposals to try to attract reviewers from other orgs.

On Fri, 11 Jan 2019 at 02:51, Julian Hyde  wrote:

> I agree with Gian.
>
> As an Apache committer, you only have one affiliation: you are working in
> the best interests of the project.
>
> Obviously, in the real world there are other pressures. But we do our best
> to compensate for them.
>
> Also, as a a community we try to design our process so as to avoid undue
> influences. For instance, when I advocate for logging cases early, I am
> trying to mitigate the effect of product managers and VPs of engineering,
> who like to have their say in meeting rooms rather than on public mailing
> lists. That’s just one example; if we see other influences at play, let’s
> evolve our process to try to level the playing field.
>
> Julian
>
>
> > On Jan 10, 2019, at 10:40 AM, Gian Merlino  wrote:
> >
> >>> What do community members think about also making a requirement that
> >>> "Design Review" PRs and proposals are reviewed by at least two people
> > with
> >>> different affiliation?
> >> This seems pretty reasonable to me. I haven't found anything in Apache
> >> voting procedure docs (https://www.apache.org/foundation/voting.html)
> that
> >> seems to explicitly forbid something like this yet at least.
> >
> > On the other hand, my understanding of the Apache Way would mean that
> this
> > kind of rule doesn't make sense. In particular from
> > https://www.apache.org/foundation/how-it-works.html: "We firmly believe
> in
> > hats. Your role at the ASF is one assigned to you personally, and is
> > bestowed on you by your peers. It is not tied to your job or current
> > employer or company." That sentiment seems incompatible with making
> > explicit rules about organizational diversity in voting. I have also
> heard
> > a few people say things like: people are supposed to represent
> themselves,
> > not their employers.
> >
> > Obviously, though, people's actions and opinions are influenced by their
> > employer. IMO, a better way to approach a potential problem there is that
> > if people from a particular organization end up behaving abusively, then
> > PMC members from other organizations (or in extreme situations the Apache
> > board itself) should tell them to knock it off. And then do more serious
> > things if the inappropriate behavior continues. This kind
> >
> > The other thing you brought up, promoting more shared ownership and
> > awareness, I am hopeful that separating proposals from PRs will help with
> > that. One reason is that it takes much less time to understand a well
> > written proposal than it takes to understand a PR. Code is very clear but
> > it is also very verbose and takes a while to read and understand. So in
> > theory changing how we operate, in this way, should promote more
> > understanding of more people across the code base.
> >
> > On Thu, Jan 10, 2019 at 1:34 AM Clint Wylie 
> wrote:
> >
> >>>
> >>> What do community members think about also making a requirement that
> >>> "Design Review" PRs and proposals are reviewed by at least two people
> >> with
> >>> different affiliation?
> >>
> >>
> >> This seems pretty reasonable to me. I haven't found anything in Apache
> >> voting procedure docs (https://www.apache.org/foundation/voting.html)
> that
> >> seems to explicitly forbid something like this yet at least.
> >>
> >> On Wed, Jan 9, 2019 at 8:52 PM Roman Leventov 
> >> wrote:
> >>
> >>> What do community members think about also making a requirement that
> >>> "Design Review" PRs and proposals are reviewed by at least two people
> >> with
> >>> different affiliation? IMO it's a good idea, because it ensures that
> >>> different interests are taken into account. Also it pushes people to
> >> engage
> >>> with work done in other parts of Druid, improving shared code owning
> and
> >>> awareness.
> >>>
> >>> Or it's against the Apache Way because it's assumed that there are no
> >>> company boundaries within the community?
> >>>
> >>> On Thu, 10 Jan 

Removal of "Coordination" tag

[Roman Leventov]

I've removed "Coordination" tag because it's always misused instead of
"Area - Segment Balancing/Coordination".

Re: Off list major development

[Roman Leventov]

In such small PRs, authors likely won't be aware that they need to create a
proposal in the first place. The first reviewer just adds the "Design
Review" tag. It's also absolutely not about considering designs and gauging
the proposal, it's just verifying that a configuration / parameter / HTTP
endpoint name is reasonable and aligned with the rest of Druid. So I think
that a separate proposal issue for such PRs is unnecessary bureaucracy.

On Tue, 15 Jan 2019 at 07:45, Jihoon Son  wrote:

> Roman,
>
> > Jihoon in
>
> https://lists.apache.org/thread.html/e007fbf362c2a870a2d88d04431789289807e00fd91d087559a01d1f@%3Cdev.druid.apache.org%3E
> and later Gian in this thread suggested that _every_ piece of work that
> should be labelled as "Design Review" according to the current rules should
> be accompanied by an issue. I don't agree with this, there are some PRs as
> small as a few dozens of lines of code, that add some configuration
> parameter and therefore should be labelled "Design Review". I don't thing a
> separate proposal issue is needed for them, and even for a little larger
> PRs too.
>
> What I'm concerned with is how people feel if their design is not accepted
> even though they wrote code. Of course, as Clint said, sometimes code helps
> better understanding of the proposal. But, I believe this is the case when
> the proposal is quite complicated and not easy to understand without code.
> Also the authors should be aware of that they might rewrite the entire code
> if the design should be changed.
>
> If writing code is simple, I don't see why the authors don't wait until the
> review for their proposal is finished.
>
> Jihoon
>
> On Fri, Jan 11, 2019 at 9:51 AM Fangjin Yang  wrote:
>
> > I agree with Gian, as an Apache committer, your responsibility is for the
> > betterment of the project. I agree it is in the best interest of the
> > project to stop thinking about what orgs people belong to. We are all a
> > part of the Apache software foundation, regardless of what our roles and
> > titles are outside of it.
> >
> > On Fri, Jan 11, 2019 at 2:22 AM Roman Leventov 
> > wrote:
> >
> > > It's not that people from one org could abuse the project and push some
> > > change, but that they have similar perspective (bubble effect) and some
> > > important aspects of a large feature could escape their attention.
> > >
> > > I suggest it to be not a rigid rule, but a recommendation for authors
> of
> > > large proposals to try to attract reviewers from other orgs.
> > >
> > > On Fri, 11 Jan 2019 at 02:51, Julian Hyde  wrote:
> > >
> > > > I agree with Gian.
> > > >
> > > > As an Apache committer, you only have one affiliation: you are
> working
> > in
> > > > the best interests of the project.
> > > >
> > > > Obviously, in the real world there are other pressures. But we do our
> > > best
> > > > to compensate for them.
> > > >
> > > > Also, as a a community we try to design our process so as to avoid
> > undue
> > > > influences. For instance, when I advocate for logging cases early, I
> am
> > > > trying to mitigate the effect of product managers and VPs of
> > engineering,
> > > > who like to have their say in meeting rooms rather than on public
> > mailing
> > > > lists. That’s just one example; if we see other influences at play,
> > let’s
> > > > evolve our process to try to level the playing field.
> > > >
> > > > Julian
> > > >
> > > >
> > > > > On Jan 10, 2019, at 10:40 AM, Gian Merlino 
> wrote:
> > > > >
> > > > >>> What do community members think about also making a requirement
> > that
> > > > >>> "Design Review" PRs and proposals are reviewed by at least two
> > people
> > > > > with
> > > > >>> different affiliation?
> > > > >> This seems pretty reasonable to me. I haven't found anything in
> > Apache
> > > > >> voting procedure docs (
> > https://www.apache.org/foundation/voting.html)
> > > > that
> > > > >> seems to explicitly forbid something like this yet at least.
> > > > >
> > > > > On the other hand, my understanding of the Apache Way would mean
> that
> > > > this
> > > > > kind of rule doesn't make sense. In particular from
> > > > > https:

The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

To foster calmness, respect, and consideration of people's busy schedules I
suggest the following etiquette:

- When someone showed up in a PR and left some review comments, but didn't
explicitly approved the PR, poke them with comments like "@username do you
have more comments?" not sooner than in one week since they left the last
review comment.
- Poke people no more frequently than once per week.
- If there are enough approvals, a PR could be merged not sooner than in
two weeks since they left the last review comment.
- If someone created a valuable PR but then stopped responding to review
comments and there are conflicts or tests/CI don't pass, a PR could be
recreated by another person not sooner than in three weeks since the
author's last activity in the PR. Their authorship should be preserved by
cherry-picking their commits, squashing them locally, rebasing on top of
current upstream master, creating a new PR and choosing "Rebase and merge"
option when merging the new PR instead of the default "Squash and merge".

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

The times that I suggested are IMO minimally reasonable for not provoking a
sense of rush and anxiety in people being poked.

If we assume that people adhere to the guideline "do not comment on a pull
request unless you are willing to follow up on the edits" from here:
https://github.com/druid-io/druid-io.github.io/blob/src/community/index.md#general-committer-guidelines
and don't forget about the PRs they started reviewing, poking appears to be
pointless. Poking and expecting the reviewer to respond "I didn't forget
about it, I'll continue my review soon" is not a good use of the time of
both people and IMO should be the common practice.

Proposal reviews are good but the code should be reviewed thoroughly too.
Despite the proposal got enough approvals, the PR with the actual code
shouldn't be rushed into the codebase.

On Fri, 25 Jan 2019 at 04:05, Gian Merlino  wrote:

> The timelines you outlined seem quite slow. Especially "if there are enough
> approvals, a PR could be merged not sooner than in two weeks since they
> left the last review comment". IMO, rather than delaying patches by so
> long, a better way to be courteous of a reviewer being too busy to review
> in a timely manner is to seek review from some other committer that has
> more time.
>
> In an extreme case, where the patch has issues that a reviewer feels must
> be addressed before the patch is merged, but the reviewer does not have
> time to hold up their end of that conversation, they can veto (
> https://www.apache.org/foundation/voting.html#Veto), accompanied by a
> justification. This is a pretty blunt tool and should not be used much. But
> it is there.
>
> I'm still optimistic that the discussion we've been having around proposals
> is a good way to address a desire for reviewers to have their say, in a way
> that doesn't slow down the development process so much. By starting
> conversations about changes earlier, it is a way for contributors to come
> together and agree on the general shape of changes before there is a bunch
> of code to discuss. Hopefully that also makes code review more efficient in
> terms of contributors' time, reviewers' time, and amount of calendar days
> that PRs are open for.
>
> On Thu, Jan 24, 2019 at 3:41 AM Roman Leventov 
> wrote:
>
> > To foster calmness, respect, and consideration of people's busy
> schedules I
> > suggest the following etiquette:
> >
> > - When someone showed up in a PR and left some review comments, but
> didn't
> > explicitly approved the PR, poke them with comments like "@username do
> you
> > have more comments?" not sooner than in one week since they left the last
> > review comment.
> > - Poke people no more frequently than once per week.
> > - If there are enough approvals, a PR could be merged not sooner than in
> > two weeks since they left the last review comment.
> > - If someone created a valuable PR but then stopped responding to review
> > comments and there are conflicts or tests/CI don't pass, a PR could be
> > recreated by another person not sooner than in three weeks since the
> > author's last activity in the PR. Their authorship should be preserved by
> > cherry-picking their commits, squashing them locally, rebasing on top of
> > current upstream master, creating a new PR and choosing "Rebase and
> merge"
> > option when merging the new PR instead of the default "Squash and merge".
> >
>

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

On Fri, 25 Jan 2019 at 23:12, Gian Merlino  wrote:

> If enough other committers have already reviewed and accepted a patch, I
> don't think it's fair to the author or to those other reviewers for
> committing to be delayed by two weeks because another committer doesn't
> have time to finish reviewing, but wants others to wait for them anyway.


It sounds like it's implied that the reviewer is irresponsible because he
started reviewing a PR but "doesn't have time to finish reviewing". In
fact, a reviewer could *have* time to finish reviewing and is willing to
finish the review, but they don't have time *tomorrow*. A reviewer could
have different duties and could slice only Y hours for reviews of Druid PRs
every X days. X/(Y * number of PRs the reviewer is interested in at the
moment) is how often (in days) a reviewer could pay attention to specific
PR. I think we should respect that for some people, at least at some times,
this value could grow to about 7. Saying that we shouldn't wait for those
people creates a bias favoring most involved developers, and it's not
necessarily a good bias, because sometimes outsider (or half-outsider)
perspective is valuable.

If we do releases every 3 months and the time between creating a release
branch and the final release candidate is at least 3 weeks (historically),
why there is an urge to commit anything faster.

IMO the real source of unfairness in reviews is that reviewers generally
prioritize the newest PRs rather than PRs that await for reviews the
longest. The probability that somebody starts to review a PR decreases with
time, while it should increase.

Proposal to shade Guava manually in Druid

[Roman Leventov]

https://github.com/apache/incubator-druid/issues/6942

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

On Tue, 29 Jan 2019 at 00:28, Gian Merlino  wrote:

> It's a totally different situation if nobody else has reviewed a patch yet.
> In that case a reviewer reviewing things with longer cycles isn't blocking
> anything.
>

There is "Development Blocker" tag for such situations.  What do you think
if for PRs tagged "Development Blocker" the "poking period" is recommended
to be 3 working days, and a week for other PRs?

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

On Tue, 29 Jan 2019 at 01:30, Fangjin Yang  wrote:

> I disagree with Roman's suggestions. If a PR has enough votes, we should
> trust the committers approving the PR and move forward.
>

There is a specific committer who merges a PR. If this happens while it's
not made clear that somebody who left comments before doesn't have any more
comments, the whole situation looks to me more like disregard of that
person's opinion. The trust to other committers doesn't help to make the
situation look much better, IMO.

Fix concurrency in old Druid classes

[Roman Leventov]

Since the start of "Druid PR checklist review" initiative (
http://mail-archives.apache.org/mod_mbox/druid-dev/201811.mbox/%3CCAAMLo=ak_wlsvky1ptvradyzliz_znx5cgqkbdyhtjz3ztr...@mail.gmail.com%3E)
we
finalized (more or less) a block of items that concern concurrency. I've
published them as a separate blog post:
https://medium.com/@leventov/code-review-checklist-java-concurrency-49398c326154

Not only new PRs can be reviewed; it's very useful to review old code too.
Here is a project about going through old Druid classes with concurrency
and improve them: https://github.com/apache/incubator-druid/issues/7061.

Everybody is welcome to pick up some class and improve it.

Re: Dev sync

[Roman Leventov]

I agree. I (probably some other people too) don't find video syncs useful
enough to attend, but some discussions happen there that might be missed.
IMO it's better to focus on fewer communication mediums: Github issues and
dev list.

On Tue, 12 Feb 2019 at 14:41, Charles Allen 
wrote:

> I am unable to host the dev sync this week.
>
> Is anyone finding utility out of these? the dev list seems pretty active
> these days, so the legacy utility of the dev sync is very muted (this is a
> good thing). Unless people are finding specific utility out of a weekly
> video sync up, I propose it be postponed indefinitely until a need can be
> identified.
>
> Thoughts?
>

Make a regular issue template to de-emphasize "Proposals"

[Roman Leventov]

The current interface of creating an issue in Druid draws a lot of
attention to "Proposals":
[image: image.png]
I think we should at least create a "regular issue" template and make it go
higher in this list of templates to restore the balance.

Online aggregation / Interactive Query Session Protocol

[Roman Leventov]

Dusting off an idea from several years ago:
https://github.com/apache/incubator-druid/issues/7087

The compatibility rules for alert data

[Roman Leventov]

What are the compatibility rules for alert data, as in
log.makeAlert(...).addData("myData", myData).emit(). Shall the key strings
and the data format be changed only in major Druid releases?

Re: TeamCity having problems

[Roman Leventov]

There is a file ci/README_TeamCity.md in Druid tree regarding TeamCity:
https://github.com/apache/incubator-druid/blob/afb239b17ac98c73825828d3f273998c165e/ci/README_TeamCity.md

Feel free to amend it with more info.

I've CC'ed this e-mail to teamcity-supp...@jetbrains.com.

On Fri, 22 Feb 2019 at 18:46, David Glasser 
wrote:

> It would be good to document that — for my first few PRs I just assumed
> that I couldn't get a login as a random contributor.
>
> On Fri, Feb 22, 2019 at 12:56 PM Justin Borromeo  >
> wrote:
>
> > You can click the “Log in as Guest” button to access the results without
> > registering a TeamCity account.
> >
> > Justin
> >
> > > On Feb 22, 2019, at 12:55 PM, Julian Hyde  wrote:
> > >
> > > The page [1] requires a login. So, a Druid contributor has to get a
> > TeamCity account?
> > >
> > > That would be OK, if TeamCity provides a lot of value. But it raises
> the
> > bar for occasional and first-time contributors.
> > >
> > > Julian
> > >
> > > [1]
> >
> https://hub.jetbrains.com/auth/login?response_type=code&client_id=7b63a7f6-a4ca-4ad2-99d6-ecede5a769a5&redirect_uri=https:%2F%2Fteamcity.jetbrains.com%2FhubPlugin%2Flogin.html&scope=0-0-0-0-0%207b63a7f6-a4ca-4ad2-99d6-ecede5a769a5&state=%2FviewLog.html%3FbuildId%3D1998017%26buildTypeId%3DOpenSourceProjects_Druid_InspectionsPullRequests&access_type=online
> > >
> > >> On Feb 22, 2019, at 10:59 AM, Gian Merlino  wrote:
> > >>
> > >> It's one of the two integrations we have on GitHub that run to
> validate
> > >> pull requests. We have TeamCity doing static analysis; and Travis
> > running
> > >> unit tests, integration tests, and checkstyle. You can see an example
> on
> > >> this pull request here:
> > https://github.com/apache/incubator-druid/pull/7118,
> > >> under "Inspections: pull requests (Druid)".
> > >>
> > >> On Fri, Feb 22, 2019 at 10:17 AM Julian Hyde 
> wrote:
> > >>
> > >>> What is TeamCity? How does Druid use it? Is it accessible to all
> Druid
> > >>> contributors? Are all Druid contributors required to use it? Is its
> use
> > >>> documented somewhere? (I searched
> > >>> https://www.google.com/search?q=apache+druid+teamcity <
> > >>> https://www.google.com/search?q=apache+druid+teamcity> but couldn’t
> > find
> > >>> any mention.)
> > >>>
> > >>>
> > >>>
> >  On Feb 22, 2019, at 8:26 AM, Furkan KAMACI 
> > >>> wrote:
> > 
> >  Hi Gian,
> > 
> >  There maybe a problem with permissions after upgrading Apache parent
> > POM
> >  version to 21. This maybe a related issue:
> >  https://jira.apache.org/jira/browse/INFRA-10286
> > 
> >  Kind Regards,
> >  Furkan KAMACI
> > 
> >  On Fri, Feb 22, 2019 at 7:13 PM Gian Merlino 
> wrote:
> > 
> > > It looks like TeamCity is having problems since a few days ago -
> > builds
> > >>> of
> > > master and PRs are flaky. Here's the link to recent builds of
> master:
> > >
> > >
> > >>>
> >
> https://teamcity.jetbrains.com/viewType.html?buildTypeId=OpenSourceProjects_Druid_Inspections&tab=buildTypeHistoryList&branch_OpenSourceProjects_Druid=__all_branches__
> > >
> > > It looks like the Windows agents generally work fine and the Ubuntu
> > >>> agents
> > > are flaky. I can't see any recent commits that seem likely to have
> > >>> caused
> > > this so am assuming something is going wrong on the TeamCity server
> > >>> side.
> > >
> > > I looked at the logs of a few successful and a few failed builds
> and
> > > noticed the failed ones have a lot of these messages, but the
> > successful
> > > ones didn't:
> > >
> > > [Step 1/1] [  43336]   WARN - ution.rmi.RemoteProcessSupport - [RMI
> > TCP
> > > Connection(3)-127.0.0.1] WARN
> > > org.eclipse.aether.internal.impl.DefaultUpdateCheckManager - Failed
> > to
> > > create parent directories for tracking file
> > >
> > >
> > >>>
> >
> /home/teamcity/.m2/repository/org/apache/apache/21/apache-21.pom.lastUpdated
> > >
> > > I'm not sure what that might indicate or what to do about it. Maybe
> > we
> > >>> need
> > > to… clear… some caches? Anyone have any ideas?
> > >
> > >>>
> > >>>
> > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> > > For additional commands, e-mail: dev-h...@druid.apache.org
> > >
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> > For additional commands, e-mail: dev-h...@druid.apache.org
> >
> >
>

Re: Datasketches

[Roman Leventov]

There is also an important sub-project in DataSketches - Memory (currently
https://github.com/DataSketches/memory) that originated from this issue:
https://github.com/apache/incubator-druid/issues/3892 and there is a plan
to eventually move Druid from ByteBuffer to Memory, at least in some parts.

On Mon, 25 Feb 2019 at 18:04, Charles Allen 
wrote:

> Basically there are a LOT of issues and PRs that show up when searching for
> datasketches in the druid PR list:
>
> https://github.com/apache/incubator-druid/pulls?utf8=%E2%9C%93&q=datasketches
>
>
> Maybe just have a label called
>
> Area - Sketches
>
> ?
>
>
> On Mon, Feb 25, 2019 at 11:01 AM Gian Merlino  wrote:
>
> > What scope would you suggest for the label or github project?
> >
> > There seem to be discussions going on around making DataSketches HLL
> and/or
> > Quantiles more 'default' options for their respective areas -- are you
> > thinking that kind of thing?
> >
> > On Mon, Feb 25, 2019 at 9:57 AM Charles Allen
> >  wrote:
> >
> > > There are a lot of here and there discussions on how to handle
> sketching
> > /
> > > hll / histograms / other-stats, and it is getting kind of hard to keep
> > > track of them all.
> > >
> > > In addition, looks like Datasketches is in an incubating proposal stage
> > for
> > > Apache
> > >
> > >
> >
> https://urldefense.proofpoint.com/v2/url?u=http-3A__mail-2Darchives.apache.org_mod-5Fmbox_incubator-2Dgeneral_201902.mbox_-253CCA-252BUaPnt-253DUvbLr-5Fv-2D4-252BYbAmHsAM-2DGqQG-252Bb-253DgOw3BL3Cemj-252BOwSA-2540mail.gmail.com-253E&d=DwIBaQ&c=ncDTmphkJTvjIDPh0hpF_w&r=HrLGT1qWNhseJBMYABL0GFSZESht5gBoLejor3SqMSo&m=Szv_v6S3DItbN0qP2B1K4mtfj4ybBA-PVuomUFw5PBU&s=U9qD5_bDYYoUyr5SJZVa-UWB5jNabHXy51TvFHczR8E&e=
> > >
> > >
> > > I think it is important enough and wide spread enough to have a top
> level
> > > consideration within the druid project. Either a label or a "github
> > > project" or something so that things can be tracked easier.
> > >
> > > Anyone have any opinions or desires here?
> > >
> > > Thanks,
> > > Charles Allen
> > >
> >
>

"Bug Report" template and "Wish list" tag

[Roman Leventov]

I suggest renaming "Bug Report" to "Problem Report" (and not automatically
assign a "bug" tag) because sometimes the problems that are reported by
users are not the result of bugs in Druid, but, for example, misuse,
misconfiguration, use of old Druid version, etc. Druid committer assigns a
"bug" tag when he reads the issue and verifies that it's probably caused by
a bug.

I also suggest removing "Wish list" tag and not assign it in
"Feature/Change request" template automatically. Or rename it to objective
"Feature", or "Feature/Change". "Wish list" is subjunctive - who wishes
that change? The author? The community?

Re: TeamCity having problems

[Roman Leventov]

I've added Maven compilation build steps as per
https://youtrack.jetbrains.com/issue/TW-21010#focus=streamItem-27-3304712.0-0
and disabled the Agent Requirement. If TeamCity keeps being flaky the Agent
Requirement needs to be re-enabled here:
https://teamcity.jetbrains.com/admin/editRequirements.html?id=buildType:OpenSourceProjects_Druid_Inspections
and
added here:
https://teamcity.jetbrains.com/admin/editRequirements.html?id=buildType:OpenSourceProjects_Druid_InspectionsPullRequests

On Mon, 25 Feb 2019 at 16:43, Gian Merlino  wrote:

> In the meantime, since Windows builds seem to be ok, I've pinned our
> TeamCity builds to Windows agents only as an effort to work around the
> problem. It might slow them down a bit -- there's only 4 Windows agents and
> 12 Ubuntu agents -- but should hopefully stem the tide of false failures.
> If this seems to cause more trouble than it's worth it can be reverted (the
> change is in "Agent Requirements").
>
> On Fri, Feb 22, 2019 at 2:00 PM Roman Leventov 
> wrote:
>
> > There is a file ci/README_TeamCity.md in Druid tree regarding TeamCity:
> >
> >
> https://github.com/apache/incubator-druid/blob/afb239b17ac98c73825828d3f273998c165e/ci/README_TeamCity.md
> >
> > Feel free to amend it with more info.
> >
> > I've CC'ed this e-mail to teamcity-supp...@jetbrains.com.
> >
> > On Fri, 22 Feb 2019 at 18:46, David Glasser 
> > wrote:
> >
> > > It would be good to document that — for my first few PRs I just assumed
> > > that I couldn't get a login as a random contributor.
> > >
> > > On Fri, Feb 22, 2019 at 12:56 PM Justin Borromeo <
> > justin.borro...@imply.io
> > > >
> > > wrote:
> > >
> > > > You can click the “Log in as Guest” button to access the results
> > without
> > > > registering a TeamCity account.
> > > >
> > > > Justin
> > > >
> > > > > On Feb 22, 2019, at 12:55 PM, Julian Hyde 
> wrote:
> > > > >
> > > > > The page [1] requires a login. So, a Druid contributor has to get a
> > > > TeamCity account?
> > > > >
> > > > > That would be OK, if TeamCity provides a lot of value. But it
> raises
> > > the
> > > > bar for occasional and first-time contributors.
> > > > >
> > > > > Julian
> > > > >
> > > > > [1]
> > > >
> > >
> >
> https://hub.jetbrains.com/auth/login?response_type=code&client_id=7b63a7f6-a4ca-4ad2-99d6-ecede5a769a5&redirect_uri=https:%2F%2Fteamcity.jetbrains.com%2FhubPlugin%2Flogin.html&scope=0-0-0-0-0%207b63a7f6-a4ca-4ad2-99d6-ecede5a769a5&state=%2FviewLog.html%3FbuildId%3D1998017%26buildTypeId%3DOpenSourceProjects_Druid_InspectionsPullRequests&access_type=online
> > > > >
> > > > >> On Feb 22, 2019, at 10:59 AM, Gian Merlino 
> wrote:
> > > > >>
> > > > >> It's one of the two integrations we have on GitHub that run to
> > > validate
> > > > >> pull requests. We have TeamCity doing static analysis; and Travis
> > > > running
> > > > >> unit tests, integration tests, and checkstyle. You can see an
> > example
> > > on
> > > > >> this pull request here:
> > > > https://github.com/apache/incubator-druid/pull/7118,
> > > > >> under "Inspections: pull requests (Druid)".
> > > > >>
> > > > >> On Fri, Feb 22, 2019 at 10:17 AM Julian Hyde 
> > > wrote:
> > > > >>
> > > > >>> What is TeamCity? How does Druid use it? Is it accessible to all
> > > Druid
> > > > >>> contributors? Are all Druid contributors required to use it? Is
> its
> > > use
> > > > >>> documented somewhere? (I searched
> > > > >>> https://www.google.com/search?q=apache+druid+teamcity <
> > > > >>> https://www.google.com/search?q=apache+druid+teamcity> but
> > couldn’t
> > > > find
> > > > >>> any mention.)
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>>> On Feb 22, 2019, at 8:26 AM, Furkan KAMACI <
> > furkankam...@gmail.com>
> > > > >>> wrote:
> > > > >>>>
> > > > >>>> Hi Gian,
> > > > >>>>
> > > > >>>> There maybe a problem with permissi

Re: "Bug Report" template and "Wish list" tag

[Roman Leventov]

Code contributions are PRs, they could be filtered via is:pr (or is:issue
if you want the opposite) in issue search on Github, isn't that enough?

On Mon, 25 Feb 2019 at 23:03, Jonathan Wei  wrote:

> "Problem Report" sounds fine to me, maybe we could have an automatically
> assigned "Problem Report" tag and a separate "Bug" tag. In general I want
> to have [TAG] or github labels on issues for easy identification.
>
> For the feature requests, I think they should be tagged automatically for
> easy visibility, and you make a fair point re: "Wish list". I would suggest
> "Feature/Change Request" as the label text (or some other word with similar
> meaning as "request" if that's too similar to "pull request"), to make it
> clear that those issues are not code contributions.
>
> Thanks,
> Jon
>
> On Mon, Feb 25, 2019 at 2:24 PM Roman Leventov 
> wrote:
>
> > I suggest renaming "Bug Report" to "Problem Report" (and not
> automatically
> > assign a "bug" tag) because sometimes the problems that are reported by
> > users are not the result of bugs in Druid, but, for example, misuse,
> > misconfiguration, use of old Druid version, etc. Druid committer assigns
> a
> > "bug" tag when he reads the issue and verifies that it's probably caused
> by
> > a bug.
> >
> > I also suggest removing "Wish list" tag and not assign it in
> > "Feature/Change request" template automatically. Or rename it to
> objective
> > "Feature", or "Feature/Change". "Wish list" is subjunctive - who wishes
> > that change? The author? The community?
> >
>

Uninterruptibles and Futures.getUnchecked()

[Roman Leventov]

I've recently discovered two utilities in Guava that are very useful in
combating InterruptedExceptions that contaminate business logic of code:

 - Uninterruptibles:
https://google.github.io/guava/releases/21.0/api/docs/com/google/common/util/concurrent/Uninterruptibles.html.
It has methods that substitute typical interruptible blocking code with a
loop that ignores an InterruptedException, as recommended in the "Java
Concurrency in Practice" book. It's recommended to try to resort to this
utility when you don't expect interruption or don't want to deal with it
and do something like wrapping into a RuntimeException or logging to get
rid of it.
 Futures.getUnchecked():
https://google.github.io/guava/releases/21.0/api/docs/com/google/common/util/concurrent/Futures.html#getUnchecked-java.util.concurrent.Future-
similar to Uninterruptibles, but additionally "washes" a Future.get() call
off ExecutionException and a CancellationException.

Re: Auto-closing old PRs

[Roman Leventov]

IMO, 60 days is nothing in Druid terms. I suggest making it 6 months.

On Thu, 28 Feb 2019 at 07:36, Dylan Wylie  wrote:

> Infra got this switched on this morning for the repository, anyone who gets
> email notifications would have unfortunately been spammed as the bot worked
> through all our old PRs. This will likely happen again in 7 days when it
> closes all the PRs that remain inactive.
>
> For anyone wanting to clean up those mails the following search string
> should take return all those mails in GMail for bulk operations
>
> "from:(stale[bot]) apache/incubator-druid"
>
> On Mon, 11 Feb 2019 at 22:15, Gian Merlino  wrote:
>
> > IMO it makes sense to keep PRs open if they have a milestone or have a
> > Security or Bug label. 60 days with no activity as a threshold sounds
> good
> > to me - it's a pretty long time.
> >
> > On Mon, Feb 11, 2019 at 11:22 AM Jihoon Son 
> wrote:
> >
> > > Hi Dylan, thank you for starting a discussion.
> > >
> > > I think this is a good idea. We currently have 159 open PRs, but many
> PRs
> > > have gone too stale. For example, the earliest PR was opened on Jan 26,
> > > 2016.
> > > I do believe that this would help us to focus on more active PRs and
> > > encourage more people to get involved in the review process.
> > >
> > > The policy for the timeline looks good to me. But, for milestone, we
> can
> > > assign it on any PRs and remove it later if it shouldn't block the
> > release.
> > > (See
> > >
> > >
> >
> https://lists.apache.org/thread.html/371ffb06447debb93eec01863802aab13a08a9c37356466e6750c007@%3Cdev.druid.apache.org%3E
> > > and
> > >
> > >
> >
> https://lists.apache.org/thread.html/b9cd3aaf2d01801751f16ee0b2beb2cebc39e2a42160ffb268dc6918@%3Cdev.druid.apache.org%3E
> > > for the discussion of the milestone policy).
> > >
> > > I think we should make bug PRs to be not auto-closed rather than the
> ones
> > > assigned a milestone.
> > >
> > > Best,
> > > Jihoon
> > >
> > > On Mon, Feb 11, 2019 at 8:27 AM Dylan Wylie 
> > wrote:
> > >
> > > > Hey folks,
> > > >
> > > > What are opinions on automatically closing old pull requests?
> > > >
> > > > There's a lot that our outdated and abandoned. I think some sort of
> > > > automated process will tidy away those that are truly abandoned while
> > > > highlighting those that aren't by encouraging their authors to poke
> > > > committers for review.
> > > >
> > > > I've taken Apache Beam's stalebot configuration and adjusted it
> > slightly
> > > > here - https://github.com/apache/incubator-druid/pull/7031
> > > >
> > > > This will:
> > > > - Leave a comment and mark PRs as stale when they haven't had any
> > > activity
> > > > for 60 days.
> > > > - After a further 7 days of no activity the PR will be closed.
> > > > - Ignore any PR that has the label "Security" or a milestone
> assigned.
> > > >
> > > > I've left issues out for now but open to suggestions on the timelines
> > for
> > > > those if we were to enact a similar process.
> > > >
> > > > Best regards,
> > > > Dylan
> > > >
> > >
> >
>

Re: Uninterruptibles and Futures.getUnchecked()

[Roman Leventov]

There are about 90 usages of Future.get() (both forms) in the codebase. I
didn't check any of them (except just one - see below), but I suspect that
in a lot of those places Futures.getUnchecked() could have been called to
clarify or even improve the logic.

That one occurrence that I've run into was
https://github.com/apache/incubator-druid/blob/a0afd7931d542fdd7149f0b02c0007e3e1fa8c65/server/src/main/java/org/apache/druid/server/coordinator/CostBalancerStrategy.java#L234-L246
- I thought that particular way to handle exceptions from a Future is bad,
started to look for alternatives, and stumbled upon Futures.getUnchecked().

There are zero usages of Uninterruptibles and Futures.getUnchecked() in the
repository so far (Uninterruptibles will first appear here:
https://github.com/apache/incubator-druid/pull/7038/files#diff-95b2c7998c679f4ebc744597fc699fc5R491),
I suspect, because nobody among Druid developers is aware of those tools
(as well as I wasn't aware of them 2 weeks ago.) So I wrote this message to
raise the developers' awareness about such tools at their hand if they need
them instead of writing next `catch (InterruptedException e) { throw new
RuntimeException(e); }`

I think this is a good idea to write such knowledge-sharing messages and
would like to see them coming from other people too.

On Thu, 28 Feb 2019 at 20:07, Gian Merlino  wrote:

> Have you got sections in mind in Druid code that would be improved by using
> these?
>
> On Tue, Feb 26, 2019 at 3:04 PM Roman Leventov 
> wrote:
>
> > I've recently discovered two utilities in Guava that are very useful in
> > combating InterruptedExceptions that contaminate business logic of code:
> >
> >  - Uninterruptibles:
> >
> >
> https://google.github.io/guava/releases/21.0/api/docs/com/google/common/util/concurrent/Uninterruptibles.html
> > .
> > It has methods that substitute typical interruptible blocking code with a
> > loop that ignores an InterruptedException, as recommended in the "Java
> > Concurrency in Practice" book. It's recommended to try to resort to this
> > utility when you don't expect interruption or don't want to deal with it
> > and do something like wrapping into a RuntimeException or logging to get
> > rid of it.
> >  Futures.getUnchecked():
> >
> >
> https://google.github.io/guava/releases/21.0/api/docs/com/google/common/util/concurrent/Futures.html#getUnchecked-java.util.concurrent.Future-
> > similar to Uninterruptibles, but additionally "washes" a Future.get()
> call
> > off ExecutionException and a CancellationException.
> >
>

Re: TeamCity having problems

[Roman Leventov]

 since it wouldn't be needed on the
> > > Druid
> > > > > > side).
> > > > > >
> > > > > > On Tue, Feb 26, 2019 at 5:33 PM Jonathan Wei 
> > > > wrote:
> > > > > >
> > > > > > > The Windows agents don't work now since we have non-Windows
> > > > executables
> > > > > > in
> > > > > > > the build process:
> > > > > > >
> > > > > > > ```
> > > > > > > Cannot run program
> > > > > > > "Z:\buildAgent\work\9f653ecb5b5406e8\web-console\script\build"
> > (in
> > > > > > > directory "Z:\buildAgent\work\9f653ecb5b5406e8\web-console"):
> > > > > > CreateProcess
> > > > > > > error=193, %1 is not a valid Win32 application
> > > > > > > ```
> > > > > > >
> > > > > > > The ubuntu agents do get past that, but Twitter's maven repo is
> > > > having
> > > > > > > issues right now so the build fails.
> > > > > > > ```
> > > > > > >
> > > > > > > [*ERROR*] Failed to execute goal on project
> > > druid-thrift-extensions:
> > > > > > *Could
> > > > > > > not resolve dependencies for project
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.druid.extensions.contrib:druid-thrift-extensions:jar:0.14.0-incubating-SNAPSHOT:
> > > > > > > Failed to collect dependencies at
> > > > > > > com.twitter.elephantbird:elephant-bird-core:jar:4.8 ->
> > > > > > > com.hadoop.gplcompression:hadoop-lzo:jar:0.4.19*: Failed to
> read
> > > > > artifact
> > > > > > > descriptor for com.hadoop.gplcompression:hadoop-lzo:jar:0.4.19:
> > > Could
> > > > > not
> > > > > > > transfer artifact
> com.hadoop.gplcompression:hadoop-lzo:pom:0.4.19
> > > > > from/to
> > > > > > > twitter (http://maven.twttr.com): Failed to transfer file:
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> http://maven.twttr.com/com/hadoop/gplcompression/hadoop-lzo/0.4.19/hadoop-lzo-0.4.19.pom
> > > > > > > .
> > > > > > > Return code is: 503 , ReasonPhrase:Service Temporarily
> > Unavailable.
> > > > ->
> > > > > > > *[Help
> > > > > > > 1]*
> > > > > > > ```
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Tue, Feb 26, 2019 at 1:28 PM Roman Leventov <
> > > > leventov...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > I've added Maven compilation build steps as per
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://youtrack.jetbrains.com/issue/TW-21010#focus=streamItem-27-3304712.0-0
> > > > > > > > and disabled the Agent Requirement. If TeamCity keeps being
> > flaky
> > > > the
> > > > > > > Agent
> > > > > > > > Requirement needs to be re-enabled here:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://teamcity.jetbrains.com/admin/editRequirements.html?id=buildType:OpenSourceProjects_Druid_Inspections
> > > > > > > > and
> > > > > > > > added here:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://teamcity.jetbrains.com/admin/editRequirements.html?id=buildType:OpenSourceProjects_Druid_InspectionsPullRequests
> > > > > > > >
> > > > > > > > On 

Druid configuration documentation should not be organized in tables

[Roman Leventov]

Rows in tables in markdown must fit a single line (unless you use
full-blown HTML  syntax), including the configuration option
descriptions. Descriptions should properly be large, sometimes even
multi-paragraph texts with a discussion of how the parameter should be
used, how lowering and increasing the value affects the system, etc.
Instead, currently many configuration option descriptions in Druid consist
of a single sentence that doesn't explain much, if anything, beyond the
configuration option's name itself.

Instead, I suggest that configuration options are titled sections in the
documentation each on its own, for example:

### `druid.coordinator.someAwesomeOption`

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
commodo consequat.

Duis aute irure dolor in reprehenderit in voluptate velit esse cillum
dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

*Default value:* `false`

*Required:* no

### `druid.coordinator.nextAwesomeOption`

...

https://github.com/apache/incubator-druid/issues/4861 would fix this
problem too.

Re: Druid configuration documentation should not be organized in tables

[Roman Leventov]

On Mon, 4 Mar 2019 at 21:36, Gian Merlino  wrote:

> I'm a bit scared that doing this would make the already large list of
> configuration options (
> http://druid.io/docs/latest/configuration/index.html)
> become even more daunting and difficult to wrap one's head around.
>

Sounds like a good nudge for ourselves to always seek for opportunities to
remove configurations! Besides, we may reorganize the page to show only
configuration names, and then a full description is open (like a "spoiler"
but not exactly, I'm not sure how this HTML control is called. Similar to
how FAQ lists are organized sometimes) when the option name is clicked.
Markdown->HTML converter that we use may support this, so the doc may need
to become a proper HTML page.

Since, so often, multiple parameters interact with each other, maybe it
> would make sense for the larger explanatory texts to be written in their
> own sections, with the parameter tables linking to them? It makes sense
> with processing buffer size and num threads configs, for example, since
> they aren't just important alone but they do relate to each other as well.
>

I'm afraid the necessity to find a place for a section, inner feeling that
a section should be "big enough" to justify its existence, the necessity of
inter-linking in markdown will make it even less likely in practice that
developers will create good docs with comprehensive discussions of
configuration options and their values.

The pull-request template

[Roman Leventov]

I've created a PR introducing the pull-request template, please check:
https://github.com/apache/incubator-druid/pull/7206

Re: Druid configuration documentation should not be organized in tables

[Roman Leventov]

I've created an issue about this problem:
https://github.com/apache/incubator-druid/issues/7210

On Tue, 5 Mar 2019 at 20:36, Eyal Yurman 
wrote:

> Just for comparison, this looks nice, isn't it?
> https://spark.apache.org/docs/latest/configuration.html
> Original .md:
> https://github.com/apache/spark/blob/master/docs/configuration.md
>
> On Tue, Mar 5, 2019 at 3:20 PM Clint Wylie  wrote:
>
> > I definitely agree that a lot of these configuration option docs have
> > outgrown the tables. I do however think the giant table as a reference to
> > lookup configuration property names and default values is handy; I think
> > maybe a system of links in the tables, like "see coordinator balancing
> docs
> > for more details" or whatever, where the implications of settings could
> be
> > described in greater detail might be more appropriate than embedding all
> of
> > the information in the tables. I do like the idea of like nested hidden
> > larger descriptions so you can sort of have both a table and a fly out
> > large description to keep it similar to what we have and not have to
> update
> > or add documentation in multiple places, but I have no idea how that
> works
> > in markdown world or if it's possible.
> >
> > On Tue, Mar 5, 2019 at 3:04 PM Roman Leventov 
> > wrote:
> >
> > > On Mon, 4 Mar 2019 at 21:36, Gian Merlino  wrote:
> > >
> > > > I'm a bit scared that doing this would make the already large list of
> > > > configuration options (
> > > > http://druid.io/docs/latest/configuration/index.html)
> > > > become even more daunting and difficult to wrap one's head around.
> > > >
> > >
> > > Sounds like a good nudge for ourselves to always seek for opportunities
> > to
> > > remove configurations! Besides, we may reorganize the page to show only
> > > configuration names, and then a full description is open (like a
> > "spoiler"
> > > but not exactly, I'm not sure how this HTML control is called. Similar
> to
> > > how FAQ lists are organized sometimes) when the option name is clicked.
> > > Markdown->HTML converter that we use may support this, so the doc may
> > need
> > > to become a proper HTML page.
> > >
> > > Since, so often, multiple parameters interact with each other, maybe it
> > > > would make sense for the larger explanatory texts to be written in
> > their
> > > > own sections, with the parameter tables linking to them? It makes
> sense
> > > > with processing buffer size and num threads configs, for example,
> since
> > > > they aren't just important alone but they do relate to each other as
> > > well.
> > > >
> > >
> > > I'm afraid the necessity to find a place for a section, inner feeling
> > that
> > > a section should be "big enough" to justify its existence, the
> necessity
> > of
> > > inter-linking in markdown will make it even less likely in practice
> that
> > > developers will create good docs with comprehensive discussions of
> > > configuration options and their values.
> > >
> >
>

TeamCity CI - new IntelliJ version

[Roman Leventov]

I've updated inspections build step in TeamCity CI to use IntelliJ 2018.3.1
instead of 2017.2.4. The builds are expected to start to fail because
IntelliJ has likely refined some existing inspections so that they make
more findings.

Re: Druid configuration documentation should not be organized in tables

[Roman Leventov]

The control I didn't know the name of is "collapsible" and it's even
supported on Github:
https://gist.github.com/joyrexus/16041f2426450e73f5df9391f7f7ae5f

On Thu, 7 Mar 2019 at 20:13, Roman Leventov  wrote:

> I've created an issue about this problem:
> https://github.com/apache/incubator-druid/issues/7210
>
> On Tue, 5 Mar 2019 at 20:36, Eyal Yurman
>  wrote:
>
>> Just for comparison, this looks nice, isn't it?
>> https://spark.apache.org/docs/latest/configuration.html
>> Original .md:
>> https://github.com/apache/spark/blob/master/docs/configuration.md
>>
>> On Tue, Mar 5, 2019 at 3:20 PM Clint Wylie  wrote:
>>
>> > I definitely agree that a lot of these configuration option docs have
>> > outgrown the tables. I do however think the giant table as a reference
>> to
>> > lookup configuration property names and default values is handy; I think
>> > maybe a system of links in the tables, like "see coordinator balancing
>> docs
>> > for more details" or whatever, where the implications of settings could
>> be
>> > described in greater detail might be more appropriate than embedding
>> all of
>> > the information in the tables. I do like the idea of like nested hidden
>> > larger descriptions so you can sort of have both a table and a fly out
>> > large description to keep it similar to what we have and not have to
>> update
>> > or add documentation in multiple places, but I have no idea how that
>> works
>> > in markdown world or if it's possible.
>> >
>> > On Tue, Mar 5, 2019 at 3:04 PM Roman Leventov 
>> > wrote:
>> >
>> > > On Mon, 4 Mar 2019 at 21:36, Gian Merlino  wrote:
>> > >
>> > > > I'm a bit scared that doing this would make the already large list
>> of
>> > > > configuration options (
>> > > > http://druid.io/docs/latest/configuration/index.html)
>> > > > become even more daunting and difficult to wrap one's head around.
>> > > >
>> > >
>> > > Sounds like a good nudge for ourselves to always seek for
>> opportunities
>> > to
>> > > remove configurations! Besides, we may reorganize the page to show
>> only
>> > > configuration names, and then a full description is open (like a
>> > "spoiler"
>> > > but not exactly, I'm not sure how this HTML control is called.
>> Similar to
>> > > how FAQ lists are organized sometimes) when the option name is
>> clicked.
>> > > Markdown->HTML converter that we use may support this, so the doc may
>> > need
>> > > to become a proper HTML page.
>> > >
>> > > Since, so often, multiple parameters interact with each other, maybe
>> it
>> > > > would make sense for the larger explanatory texts to be written in
>> > their
>> > > > own sections, with the parameter tables linking to them? It makes
>> sense
>> > > > with processing buffer size and num threads configs, for example,
>> since
>> > > > they aren't just important alone but they do relate to each other as
>> > > well.
>> > > >
>> > >
>> > > I'm afraid the necessity to find a place for a section, inner feeling
>> > that
>> > > a section should be "big enough" to justify its existence, the
>> necessity
>> > of
>> > > inter-linking in markdown will make it even less likely in practice
>> that
>> > > developers will create good docs with comprehensive discussions of
>> > > configuration options and their values.
>> > >
>> >
>>
>

Re: TeamCity CI - new IntelliJ version

[Roman Leventov]

I've rolled back to 2017.2.4. Failing builds (e. g.
https://teamcity.jetbrains.com/viewLog.html?buildId=2045510&tab=Inspection&buildTypeId=OpenSourceProjects_Druid_InspectionsPullRequests)
provide enough information to try to fix the problems and then to try to
upgrade to 2018.3.1 again.

Independent of that, the master build is still failing:
https://teamcity.jetbrains.com/viewType.html?buildTypeId=OpenSourceProjects_Druid_Inspections&tab=buildTypeHistoryList.
One may not pay attention to that as long as PR builds are not failing, but
the "TeamCity inspection" badge in Druid's README says "invalid response
data".

On Fri, 8 Mar 2019 at 22:54, Gian Merlino  wrote:

> Or at least do some spot checks to verify that the TC errors are not
> related to the patch in question.
>
> On Fri, Mar 8, 2019 at 5:50 PM Gian Merlino  wrote:
>
> > That sounds fine to me (ignoring TC for now on other PRs while any new
> > issues since the upgrade is fixed separately). If no-one is able to fix
> > them quickly I'd suggest rolling back to 2017.2.4, so we don't have a
> super
> > long period of time where we need to ignore TC.
> >
> > On Fri, Mar 8, 2019 at 5:29 PM Jihoon Son  wrote:
> >
> >> This blocks all existing PRs from being merged.
> >> I don't think this is important enough to block all PRs.
> >> What do you think about opening an issue to fix this in one place?
> >> In the meantime, we can ignore teamcity inspection result.
> >>
> >> Jihoon
> >>
> >> On Fri, Mar 8, 2019 at 2:58 PM Gian Merlino  wrote:
> >>
> >> > Please help fix anything that breaks. Hopefully this also _improves_
> >> things
> >> > -- I recall an inspection bug we hit that was fixed in some version
> >> later
> >> > than 2017.2.4.
> >> >
> >> > On Fri, Mar 8, 2019 at 12:21 PM Roman Leventov 
> >> > wrote:
> >> >
> >> > > I've updated inspections build step in TeamCity CI to use IntelliJ
> >> > 2018.3.1
> >> > > instead of 2017.2.4. The builds are expected to start to fail
> because
> >> > > IntelliJ has likely refined some existing inspections so that they
> >> make
> >> > > more findings.
> >> > >
> >> >
> >>
> >
>

About [tags] in issue and PR headers

[Roman Leventov]

1) Do other people want proposal issues to have [PROPOSAL] in their names,
despite they also have a corresponding tag? Maybe we can at least make it
not caps?

2) Would be nice to teach bot to visit PRs and issues from non-committers
and remove "[tag name]" parts from their titles and assign corresponding
tags.

Re: PR Milestone policy

[Roman Leventov]

Looks like three committers (Jonathan, Mingming and I) voted for the policy
of not adding non bug and security PRs to milestones before merging.
One (Gian) abstained. Therefore I included this in the PR action item list
for committers: https://github.com/apache/incubator-druid/pull/7279 please
review.

On Mon, 7 Jan 2019 at 16:11, Gian Merlino  wrote:

> My feeling is that setting a milestone on PRs before they're merged is a
> way of making their authors feel more included. I don't necessarily see a
> problem with setting milestones optimistically and then, when a release
> branch is about to be cut (based on the timed release date), we bulk-update
> anything that hasn't been merged yet to the next milestone.
>
> However, there are other ways to make authors feel more included. If we end
> up doing a more formalized proposal process then this helps too. (It should
> be easier for people to comment on proposals than on PRs, since there isn't
> a need to read code.)
>
> I guess I'm not really fussy either way on this one.
>
> On Wed, Dec 12, 2018 at 10:27 PM 邱明明  wrote:
>
> > I agree with Jonathan.
> > Jay Nash  于2018年12月13日周四 下午1:05写道：
> > >
> > > Dear all,
> > > I am just bystander on Druid List however I like to contribute code to
> > Druids some day because it is very great, we use it at my company. It
> > sounds consensus was reached that Github milestones should be used not so
> > frequently and is proposed vote about to change this.. is this correct?
> > >
> > > Regards,
> > > Jay
> > >
> > > On 2018/12/12 00:39:29, Jonathan Wei  wrote:
> > > > After a PR has been reviewed and merged, I think we should tag it
> with
> > the>
> > > > upcoming milestone to make life easier for release managers, for all
> > PRs.>
> > > >
> > > > Regarding unresolved PRs:>
> > > >
> > > > > I advocate for not assigning milestones to any non-bug (or
> otherwise>
> > > > "critical") PRs, including "feature", non-refactoring PRs.>
> > > >
> > > > That seems like a reasonable policy to me, based on the points Roman
> > made>
> > > > in the thread.>
> > > >
> > > > On Tue, Dec 11, 2018 at 1:13 AM Julian Hyde 
> wrote:>
> > > >
> > > > > Well, see if you can get consensus around such a policy. Other
> Druid>
> > > > > folks, please speak up if you agree or disagree.>
> > > > >>
> > > > > > On Dec 8, 2018, at 8:02 AM, Roman Leventov >
> > > > > wrote:>
> > > > > >>
> > > > > > It's not exactly and not only that. I advocate for not assigning>
> > > > > milestones>
> > > > > > to any non-bug (or otherwise "critical") PRs, including
> "feature",>
> > > > > > non-refactoring PRs.>
> > > > > >>
> > > > > > On Fri, 7 Dec 2018 at 19:29, Julian Hyde 
> > wrote:>
> > > > > >>
> > > > > >> Consensus.>
> > > > > >>>
> > > > > >> We resolve debates by going into them knowing that we need to
> > find>
> > > > > >> consensus. A vote is a last step to prove that consensus exists,
> > and>
> > > > > >> in most cases is not necessary.>
> > > > > >>>
> > > > > >> Reading between the lines, it sounds as if you and FJ have a>
> > > > > >> difference of opinion about refactoring changes. Two extreme
> > positions>
> > > > > >> would be (1) we don't accept changes that only refactor code,
> (2)
> > and>
> > > > > >> I assert my right to contribute a refactoring change at any
> point
> > in>
> > > > > >> the project lifecycle. A debate that starts with those positions
> > is>
> > > > > >> never going to reach consensus. A better starting point might be
> > "I>
> > > > > >> would like to make the following change because I believe it
> > would be>
> > > > > >> beneficial. How could I best structure it / time it to minimize>
> > > > > >> impact?">
> > > > > >> On Fri, Dec 7, 2018 at 9:19 AM Roman Leventov  >>
> > > > > >> wrote:>
> > > > > >>>>
> > >

Re: Druid module separation (And a related lookups question)

[Roman Leventov]

Hadoop and Spark batch indexing are the only contexts (I think) where only
'processing' is needed, but not 'server'.

The separation between 'core' and 'processing' is a historical artifact
that should be fixed, see
https://github.com/apache/incubator-druid/issues/4312#issuecomment-356127914

Also, when it is done, it would be nice to check whether each utility class
that is currently in 'core' is used in 'processing' (or new 'core', if
'processing' is renamed to 'core'), or only in 'server' and elsewhere. In
the latter case, the utility class should be moved to 'server'.

On Wed, 20 Mar 2019 at 18:59, Charles Allen 
wrote:

> First, note that Server depends on Processing.
>
> Most of the things in Core are used everywhere and are underlying
> utilities. This has taken on various names like "common" and "util" in the
> past.
>
> Most of the things in Processing are there to facilitate the execution of a
> query.
>
> Most of the things in Server are there to facilitate the running of the
> service.
>
> There are exceptions to this, such
> as org.apache.druid.query.dimension.LookupDimensionSpec being in Server.
> The reason is that it currently requires reference to a thing in Server to
> bind to. This should probably be undone (via an interface) at some point
> but has not been a priority.
>
> Most of the other look up stuff in Server is about the metadata management
> and loading if I recall.
>
>
>
> On Wed, Mar 20, 2019 at 2:51 PM Eyal Yurman
>  wrote:
>
> > Hi,
> >
> > Can someone help me understand the logic for the separation of core,
> server
> > and processing modules, and why?
> >
> > More specificly, I'm trying to understand the split of the package
> > `org.apache.druid.query.lookup` between the two latter modules.
> >
> > Eyal.
> >
>

Policy for accepting new modules and large contributions. Protocol for removing modules

[Roman Leventov]

Historically, Druid has been exceptionally inclusive for external
contributors, but support costs are not taken into account.

Does anybody see this as a problem?

We may require for large "feature" PRs to core modules, including PRs into
some "core" extensions (such as kafka-indexing and sql) (those that
increase number of classes and complexity) to either
 - Be authored by a committer. So if a non-committer want to push such a
big change to the core, he needs to become a committer first.
 - Be "endorsed" by at least two committers who are recently active in
Druid. (Read: supporting Druid is part of their job duty, at least
part-time.)

Regarding removing old modules, we may revisit all extensions in
extensions-core and extensions-contrib.
 - The new contrib may not be tested in CI builds unless the contrib code
is changed in the PR (note: I failed to quickly find if Travis supports
such thing. It may not.)
 - Contrib modules may not be included in the general Druid distribution
(but some vendors may include some contrib modules into their
distributions).
 - Updating contrib modules' code (even making them compile against the
core) may become the sole responsibility of people who care about these
contrib modules.
 - If we see that some contrib module "falls off" and nobody updates it
for, say, one year, we remove the module from the codebase completely.

Note: both ideas presented above are merely ideas, not firm proposals.

Adding IntelliJ's structural search pattern as an error for TeamCity

[Roman Leventov]

I've written a tutorial about how to add an IntelliJ's structural
search pattern as an error for TeamCity builds:
https://github.com/apache/incubator-druid/blob/a4270da5f3de25f1182cfdf2ac0d9de7781312cf/ci/README_TeamCity.md#creating-a-custom-inspection-from-a-structural-search-pattern

If somebody wants to try it out, you can fix one of the following issues
using this technique:
 - https://github.com/apache/incubator-druid/issues/7316
 - https://github.com/apache/incubator-druid/issues/6704

Working out principles for choosing logging severity

[Roman Leventov]

Please comment: https://github.com/apache/incubator-druid/issues/7362

Please vote for several IntelliJ/TeamCity issues

[Roman Leventov]

These issues directly block our upgrade to 2018.3.1 in TeamCity:
 https://youtrack.jetbrains.com/issue/IDEA-209789
 https://youtrack.jetbrains.com/issue/IDEA-209791

Also, these two issues fixed would be very helpful for Druid CI otherwise:
 https://youtrack.jetbrains.com/issue/TW-59220
 https://youtrack.jetbrains.com/issue/TW-46893

Please log in and vote (a small thumbs up button) for these issues. It's
known to be not completely pointless, JetBrains employees choose the next
issues to fix by the numbers of votes.

Re: Coordinator and Overlord Readiness Checks

[Roman Leventov]

There is a PR which adds a very similar thing (/selfDiscovered endpoint)
that could probably be used for your purpose:
https://github.com/apache/incubator-druid/pull/6702.

On Fri, 5 Apr 2019 at 23:23, Julian Jaffe 
wrote:

> Hey all,
>
> I'm looking to add readiness checks to coordinator and overlord nodes to
> improve automatic deployment of clusters. More specifically, I'm planning
> to add an endpoint to the coordinator and overlord that returns 200 OK when
> the node is ready to process lookup/tiering rules/supervisorspec/etc.
> requests. My first thought is to add another module and bind at the
> Stage.ANNOUNCEMENT stage. Is there any downside to this approach, or were
> readiness checks for historicals implemented differently for historical
> reasons?
>
> Thanks,
> Julian
>

Domain-driven Observability

[Roman Leventov]

There is a recent discussion of logging (including its code-level
noisiness): https://github.com/apache/incubator-druid/issues/7362

This article describes an interesting pattern that we can probably apply in
some parts of the Druid codebase:
https://martinfowler.com/articles/domain-oriented-observability.html

Some useful annotation from error_prone_annotations

[Roman Leventov]

It's suggested to annotate methods that throw
UnsupportedOperationException @DoNotCall, to catch calling to them in
compile time (actually, "error-prone time").

PVS-Studio static analysis

[Roman Leventov]

This bug: https://github.com/apache/incubator-druid/issues/7423 was found
thanks to PVS-Studio. Developers of Druid are recommended to obtain an
open-source license of PVS-Studio and run the analysis from time to time.
It works as a plugin for IntelliJ.

See https://www.viva64.com/en/b/0600/
https://www.viva64.com/en/open-source-license/

Re: Code review

[Roman Leventov]

Apart from code review friction, I think there is another important
(ultimately, more important) problem/goal for the long term project success
which cannot be discussed separately: keeping the codebase highly
maintainable. I'm a Druid developer for about 2.5 years, and I definitely
feel that making changes is more difficult than it was 2.5 years ago. It
means that the codebase is aging not very well. The project is on the blue,
not orange trajectory here:
https://martinfowler.com/bliki/DesignStaminaHypothesis.html.

So, regarding the part of Gian's proposal that is essentially saying "let's
lower review standards": I think the current quality of contributions is
not high enough. If we lower the review standards, as it currently goes,
the contribution quality will drop, too. The project may end up with a
great community but not being able to match the innovation pace happening
in similar other projects.

To discuss how we can make the quality of contributions higher, let's first
note that there are two types of contributions:
 - "Internal": contributions by regular contributors. Developing Druid is a
large part of their day job at their companies. I think currently it's
about 80% of all contributions, the majority of that is from employees of
Imply.
 - "External": by people who are not regular contributors. Developing Druid
is usually not part of their job.

The good news is that since the majority of contributions are coming from
the employees of companies which have a stake in the Druid's success, there
is a way to increase the quality of contributions _and_ reduce the code
review friction: ask the regular contributors to "do the right thing".

One concrete example: code style comments. Personally, I leave a lot of
comments about violations of the Druid's code style. However, I very rarely
leave such comments on Gian's PRs because he is one of the people who
created the style and follows it. So the way to reduce the friction from
such comments is to ask regular contributors to learn and follow that style
themselves. This way the number of such comments will reduce drastically,
simply because there would be nothing to comment about.

I think "the right thing" for regular contributors can be boiled down to
the following:

1. Self-review the PR before publishing, according to a set of standards
and carefully chosen self-ask questions (checklist items).

2. Communicate in PR reviews *proactively*:
   - Both the author and reviewers proactively make the life of other
parties easier, regardless of whether the other party did make their life
easy in the PR or not:
   - The author makes the description of the PR elaborate, yet
highlights changed/added APIs, important changes, design considerations,
etc. (Both Gian and Slim have mentioned that.)
   - A reviewer provides the author with necessary information about
their comments: links, etc. A reviewer doesn't ask rhetoric questions (but:
asks real questions, if they have some).
   - If a reviewer asks questions about the change because they don't
understand something about the code, the author proactively adds comments
to the code or restructures the code so that that the reviewer wouldn't
have that question in the first place.
   - If a reviewer points to some problem, the author proactively searches
for all similar problems in your PR and fix them, without waiting for a
reviewer to find them for you.
   - If either the author or a reviewer notices some side problem during
the discussion, they proactively create an issue about that problem
themselves, not waiting for the other party to do that.
   - If either the author or a reviewer discovers something new for
themselves during the discussion, they proactively internalize that new
knowledge, so that it becomes the part of their knowledge, and they will
share that knowledge with somebody else during a different PR review
discussion if needed. This knowledge may be about the code style, the
programming practice, some specific knowledge about the project. Note1: the
concrete example of code style comments from above falls down here. Note2:
if the discovered knowledge about the code style or the programming
practice is not part of the standards, it should be added there. Note3: the
specific knowledge about the project should be put somewhere in the
comments in the code itself and linked from all the places where it is also
relevant.

3. Regularly review other people's PRs, don't just publish your code. Gian
have mentioned this. Addition from me: review PRs by people from not your
org regularly.

4. Regularly dedicate time to do a PR that increases automation of the
project, for example:
- Automate something about the CI or testing
- Add a static analysis (IntelliJ) check
- Add a Checkstyle check

Gian mentioned the need to increase automation, but without committing to
regular practice by all regular contributors, I'm afraid it will remain
words.

5. Regularly dedicate time to do a PR that is de

IntelliJ Ultimate Edition license for Druid developers

[Roman Leventov]

Apache Committers can get a license for IntelliJ IDEA Ultimate without
engaging in e-mail conversations, in a matter of minutes, by using their @
apache.org e-mail: https://www.jetbrains.com/shop/eform/apache?product=ALL.
There is at least one little reason to use Ultimate for Druid development:
the "Detecting Duplicates" feature which is available only in Ultimate (see
https://www.jetbrains.com/idea/features/editions_comparison_matrix.html).

People who are not committers can also apply for an Ultimate license for
Druid development here:
https://www.jetbrains.com/shop/eform/opensource?product=ALL.

Re: IntelliJ Ultimate Edition license for Druid developers

[Roman Leventov]

I don't know, but you can ask them at opensou...@jetbrains.com and share
the result

On Wed, 24 Apr 2019 at 21:26, Himanshu  wrote:

> I regularly work on multiple projects aside from Druid (some proprietary) .
> Does this license allow us to work on *all* Java projects or is this
> restricted only to Apache projects development?
>
> I tried to understand above from the "Jetbrains Account Agreement" page but
> failed.
>
> -- Himanshu
>
> On Wed, Apr 24, 2019 at 7:33 AM Roman Leventov 
> wrote:
>
> > Apache Committers can get a license for IntelliJ IDEA Ultimate without
> > engaging in e-mail conversations, in a matter of minutes, by using their
> @
> > apache.org e-mail:
> https://www.jetbrains.com/shop/eform/apache?product=ALL
> > .
> > There is at least one little reason to use Ultimate for Druid
> development:
> > the "Detecting Duplicates" feature which is available only in Ultimate
> (see
> > https://www.jetbrains.com/idea/features/editions_comparison_matrix.html
> ).
> >
> > People who are not committers can also apply for an Ultimate license for
> > Druid development here:
> > https://www.jetbrains.com/shop/eform/opensource?product=ALL.
> >
>

Re: Code review

[Roman Leventov]

On Wed, 24 Apr 2019 at 00:10, Gian Merlino  wrote:

> Well, I think this is a good reason to have a robots-only policy for code
> style checking. The fact that I am one of the few people that can adhere to
> the unwritten style rules, because I've been working on Druid for over 7
> years, means that the system today doesn't work.
>

What do you think about switching to a different, well-established code
style? The problem with Druid's style is that neither IntelliJ, nor
Eclipse, nor Checkstyle can fully support it at the moment, and it doesn't
look to me that they will in a foreseeable future. If we choose, for
example, Java Sun style or Google's style, I think at least Checkstyle
should be able to fully cover it.

I would add one thing: for all the "authors should proactively…" stuff,
> reviewers should mention that when they leave comments. For example,
> instead of "why did you do this?" a better review comment is "I can't
> understand why you did this. Could you please add a code comment explaining
> it?" Even better, if you suspect the code is wrong: "I can't understand why
> you did this. It looks like it might be incorrect for X reason. Could you
> please look into that, and if it seems like the code is correct after all,
> add a comment explaining how it works?"
>

I agree that a reviewer should do this. It falls into the earlier point
from my list: "Both the author and reviewers proactively make the life of
other parties easier". However, the idea of that "proactive" list is that
all parties should always take the most proactive action whenever they can
regardless of whether the other party took a proactive action or not. Every
reviewer can once just ask a question and forget to add that mouthful part
about adding a comment, there will always be reviewers who won't follow
this policy or won't be aware of it, etc. We want to avoid a situation when
a reviewer just asks, the author just answers in a PR discussion, and
knowledge is not recorded in the code. When both (regular) reviewers and
authors do the proactive thing, this situation should be rare.


>
> It's a bit more work for reviewers but I think it makes a real difference
> in the experience that authors have. It minimizes back-and-forth, which
> minimizes frustration. And it helps propagate culture, since authors don't
> just see what is being asked for, but also why, and they get a sense of
> what the project's shared values are.
>
> > 3. Regularly review other people's PRs, don't just publish your code.
> Gian
> > have mentioned this. Addition from me: review PRs by people from not your
> > org regularly.
>
> +1000
>
> > 4. Regularly dedicate time to do a PR that increases automation of the
> > project, for example:
> > - Automate something about the CI or testing
> > - Add a static analysis (IntelliJ) check
> > - Add a Checkstyle check
>
> Anything we can move from manual review to robotic review is good. I would
> caution against getting overly aggressive with static analysis &
> checkstyle, but I don't think we're there yet. The checks we have don't
> seem to be excessive.
>
> > 5. Regularly dedicate time to do a PR that is devoted specifically to
> > repaying tech dept (yak shaving, refactoring)
>
> These are tough ones. Because all refactorings have risk, and because they
> are not expected to yield bug fixes or functional improvements, they tend
> to languish in the review queue. So extra work is needed by authors to
> motivate review of them. I don't do too many of these sorts of PRs myself,
> but when I do, I try to attach motivation.
>

I have the opposite opinion.
1) In my experience in Druid, a refactoring is usually associated with
fixing old hidden bugs. Consider
https://github.com/apache/incubator-druid/pull/7038, in which 5 different
concurrency bugs were fixed. I'm sure that the majority of old concurrency
classes in Druid have concurrency bugs, so doing any refactoring in this
project: https://github.com/apache/incubator-druid/projects/4 will result
in fixing bugs.
2) I think if we want to move to the good trajectory from
https://martinfowler.com/bliki/DesignStaminaHypothesis.html, _reviewing_
refactoring PRs (nb: not doing them) should be a high priority. So not
"extra work", but less work would be needed from authors to motivate
reviewers to review it. Currently, there is a negative feedback cycle:
people don't do refactoring because they know that it will take an infinite
time to merge them, hence the associated conflict resolution work will be
disproportionally huge. This is exacerbated by the fact that refactorings
tend to touch more existing files in the codebase than feature PRs, and
naturally, require more master conflict resolution work. Again: I don't say
that we should refactor all the time, but I think that we want to ensure
that when a developer decides to do some refactoring, he is confident that
it will be relatively quick experience with little master conflict
resolution work.


> I don't think 

Throwables.getRootCause()

[Roman Leventov]

Please use Throwables.getRootCause() as a robust way to get
e.getCause().getCause()... until null is encountered. I think this is
especially true in REST, where we should say

serverError().entity(ImmutableMap.of("error", Throwables.getRootCause(e)))

To provide the caller with the actual reason of the error (for example,
some I/O errors sometimes have long cause chains with something like "no
disk space left on device" in the end).

(Side note: somebody may want to inspect the security of this practice, but
currently the code is not considered with security implications of
reporting errors to REST callers at all, as far as I can say.)

(Second side note: for logging, it's probably better to not to unwind the
cause, to provide cluster operators and developers with the full stack.
However, there are some places of log.error("...", e.getCause()) in the
codebase, which should probably be replaced with just log.error("...", e).)

There are also some places in the codebase with code like (e.getCause() ==
null ? e : e.getCause()) (just search "getCause" in the codebase) which
should probably better be replaced with Throwables.getRootCause().

Re: Code review

[Roman Leventov]

A quote from Steve Tockey regarding documentation and comments and the cost
of maintaining software:
https://medium.com/@leventov/steve-tockey-on-software-documentation-and-comments-470751227a63

On Mon, 29 Apr 2019 at 19:30, Fangjin Yang  wrote:

> Strong +1 agreement with Gian. I definitely see there are anal code reviews
> focused too much on trying to force changes that make no difference to the
> core functionality being contributed, and are instead a reflection of
> personal biases of the reviewer. These types of low quality code reviews
> provide absolutely no value to the Druid community, and instead serve to
> damage the community engagement, and slow down development.
>
> On Thu, Apr 25, 2019 at 11:48 AM Julian Hyde  wrote:
>
> > +1
> >
> > As someone who reviews Druid release candidates but does not develop
> Druid
> > code, I wish that “mvn test” would “just work”. I can imagine that
> > first-time and occasional contributors are in a similar boat to me.
> >
> > I know this is not an easy thing to achieve, because the test suite also
> > has to work for more experienced contributors.
> >
> > Julian
> >
> >
> > > On Apr 25, 2019, at 11:42 AM, David Glasser  >
> > wrote:
> > >
> > > As a new contributor, I've actually really appreciated the careful and
> > high
> > > quality code reviews I've received (primarily from Jihoon).
> > >
> > > The real source of friction for me has not been the feedback from
> > > developers, but the painfulness of running the tests.
> > >
> > > - Figuring out how to locally run the full suite of tests that CI will
> > run
> > > is not super documented.
> > > - Understanding how to just run the tests that are most relevant to
> your
> > > change (either from CLI or IntelliJ) isn't well documented.  It's
> > > especially unclear what things can be perfectly successfully run from
> > > IntelliJ's own test runners and what things really require you to run
> > them
> > > through Maven.  (Druid is also the first time I've used Maven directly
> > and
> > > it's a huge challenge for me to understand what's going on and how to
> run
> > > it properly.)
> > > - Many of the tests are just very slow and maybe could be parallelized
> > > better?
> > >
> > > It's enough of a pain that I often when iterating wouldn't even bother
> > > trying to run tests manually but would just push to GitHub and let
> Travis
> > > run it instead. But Travis itself is very slow and while it's somewhat
> > > parallelized, it's not super parallelized — and as a lowly PR author I
> > > can't even kill the overall job if I push a new version of the PR or
> if a
> > > sub-job already failed.  So this would frequently add "round trip"
> times
> > of
> > > half an hour or more in the middle of trying to take the good advice
> from
> > > reviewers to heart.
> > >
> > > So while I agree that it would be great to move as many of the checks
> as
> > > possible into CI from "core dev teams mind", I'd also encourage folks
> > with
> > > more time and expertise than I have now to put effort into making the
> CI
> > > experience faster and easier.  (I wonder if some of the issues could
> just
> > > be solved with money, eg running on more powerful Travis machines or
> > > parallelizing the slower tests even further onto a larger number of
> > > containers.  I've also generally found in my own work that CircleCI at
> > > least feels faster and easier to work with than Travis FWIW.)
> > >
> > > --dave
> > >
> > > On Mon, Apr 22, 2019 at 7:44 PM Gian Merlino  wrote:
> > >
> > >> Hey Druids,
> > >>
> > >> Sometimes I feel like this too:
> > >> https://twitter.com/julianhyde/status/1108502471830204416.
> > >>
> > >> I believe our code review process today has too much friction in it,
> and
> > >> that we can work to reduce it. The two main frictions I see are code
> > >> reviews not happening in a timely manner, and code reviews sometimes
> > >> descending into a swamp of nit-picks. The good news, at least, is that
> > we
> > >> are not the first development team to have these problems, and that
> they
> > >> can be solved. I've written some thoughts below about principles that
> I
> > >> think might help. I am not necessarily proposing making these the law
> of
> > >> the land, but am hoping to start a discussion about how we can
> generally
> > >> improve things.
> > >>
> > >> 1) "Let robots handle style checks." Encode Druid's code style in
> > >> checkstyle or other tools, and avoid making subjective style comments
> > >> directly as humans. If we can't figure out how to set up automated
> > >> verification for a particular style point, then give it up. Rationale:
> > >> authors can self-check style when checking is automated. Also, it's
> > better
> > >> for robots to enforce style, because software development is a social
> > >> endeavor, and people don't mind style nit-picking from robots as much
> as
> > >> they do from humans.
> > >>
> > >> 2) "Write down what really matters." I suggest we put together a
> sho

Re: TeamCity issue

[Roman Leventov]

Also, https://youtrack.jetbrains.com/issue/TW-60512 is about our inability
to upgrade to IntelliJ 2018 (or 2019) in our inspections build.

On Tue, 21 May 2019 at 02:20, Gian Merlino  wrote:

> FYI: I just raised https://youtrack.jetbrains.com/issue/TW-60491 for what
> looks like a TeamCity issue that is affecting Druid PRs right now.
>

PRs requiring design review

[Roman Leventov]

Could some of the committers please provide a design review of REST APIs
(no line-by-line code review is strictly necessary since done by others) to
these PRs:

- https://github.com/apache/incubator-druid/pull/6702 -
SelfDiscoveryResource of Druid nodes
- https://github.com/apache/incubator-druid/pull/7653 - Refactor
SQLMetadataSegmentManager

Thanks

RESTful API Guidelines

[Roman Leventov]

Interesting project: https://opensource.zalando.com/restful-api-guidelines/

I think we can use it for Druid's REST/inter-server communication API
checklist (like with concurrency checklist, we can list items in Druid's
repo but link to Zalando's website for details).

Running integration tests

[Roman Leventov]

The instruction for running integration tests locally at
integrations-tests/README.md doesn't work for me. I think some security
setup step is missing. See
https://github.com/apache/incubator-druid/issues/7842

Moving project to Gradle

[Roman Leventov]

Please join the discussion:
https://github.com/apache/incubator-druid/issues/7866

Re: Stalebot for issues

[Roman Leventov]

What's the purpose of closing old issues? How does it make the Druid
development experience better?

On Thu, 20 Jun 2019 at 23:54, Gian Merlino  wrote:

> By the way, I do think it makes sense to have a stalebot for issues. Right
> now we have over 1000 open issues and I doubt anyone is actively reviewing
> them. IMO it would be better to keep the list shorter, to things where the
> original filer is still actively interested in keeping them alive. But I
> think 60 days is a bit short, hence the suggestion in my follow up PR to
> raise it to 280 days (about 3 release cycles).
>
> On Thu, Jun 20, 2019 at 12:40 PM Gian Merlino  wrote:
>
> > There's been some discussion on GitHub about enabling stalebot (which we
> > use for PRs) for issues as well. Please check this PR out if you are
> > interested: https://github.com/apache/incubator-druid/pull/7936. It's a
> > follow up to https://github.com/apache/incubator-druid/pull/7927, which
> > is also recent.
> >
>

Re: Stalebot for issues

[Roman Leventov]

On Fri, 21 Jun 2019 at 18:38, Gian Merlino  wrote:

> The effect should be giving us an
> open issues list that more accurately respects the issues that people in
> the community feel are important.
>

The list would still be too long to be comprehensible or digestible for
anybody, nor that anyone is expected to go through the full list at any
time.

I see the value of nudging PR authors to push their work through rather
than abandon PRs in pursuit of something new, hoping to return to the older
PRs later (which will likely never happen) - that is, to avoid this
psychological fallacy.

But I don't see the same value for issues. Personally, I open many issues
which I don't really plan to work on in any foreseeable future, just to
record my ideas and thoughts so that they can be discovered by other
developers (and myself) later, and referenced to from future discussions,
issues, and PRs. I see a real practical value in it, as I routinely link to
my own old issues (and re-read them, refreshing my old thoughts on the
topic) in Druid development. I don't want to take on a burden of regularly
repel the stalebot from all of these issues.


> As more and more work piles up, it becomes paralyzing.


What I suggest is to embrace the fact that open issues list will grow as
long as the project exists and don't be paralyzed. Why would a number in a
circle in Github interface paralyze anybody from doing work, anyway?


> Just making decisions about what work should and shouldn't get
> done can exhaust all available resources.


This statement doesn't make sense to me as well as the previous one. I
actually agree that priorities and focus is an important issue for a
project like Druid where there are a lot of directions in which it can be
improved and it's hard to choose (predict) the direction with the highest
ROI. But I don't see how going down from 1000 to 100 open issues would help
with this challenge at all.

As a compromise approach, I suggest to auto-tag issues as "Shelved",
although, personally, I don't see the point in that either, but if other
people want to see if there is any recent activity on the issue, it might
be helpful.

Re: Stalebot for issues

[Roman Leventov]

I wrote previous messages in this thread before I've discovered that the
stalebot send me more than 100 messages. (That shouldn't be surprising
since I'm the author of 174 open issues in Druid:
https://github.com/apache/incubator-druid/search?p=1&q=is%3Aopen+author%3Aleventov+is%3Aissue&type=Issues).
As an experiment, I'll try to go over all notifications and post here how
many of them can actually be closed now.

On the other hand, I've realized that a big and a legitimate case for
stalebot closing issues are the issues of "Problem report" kind (
https://github.com/apache/incubator-druid/issues/new?assignees=&template=problem_report.md&title=).
The reasoning is that
 - As time passes, the issue may be fixed in the newer Druid versions.
 - The report may be irreproducible or hardly reproducible, especially if
the Druid version used is unspecified or there is otherwise too little
information in the issue.

"Flaky test" issues are somewhat similar, too.

Jon once suggested to add a "Problem report" tag:
https://lists.apache.org/thread.html/61068635cc338dd0da6d43bfca16adf9ccdd3d61e267b598124ca3ad@%3Cdev.druid.apache.org%3E.
We could revive this idea in the form of "Uncategorized problem report". It
would be a committer's duty to reassign either to "bug", "invalid", or
"won't fix" upon verification.

Then, I suggest that the stalebot only watches "Uncategorized problem
report", "Flaky test", and issues without any tags (that would sweep all
old issues which are essentially uncategorized problem reports, as well as
new issues when the authors use the "Other" button instead of "Problem
report" button).

I think that the majority of "Feature/Change request", "Feature",
"Refactoring", "Performance", etc. issues would be "evergreen", so it's
more practically to close them only by occasion when someone visits these
old issues.

On Fri, 21 Jun 2019 at 21:57, Gian Merlino  wrote:

> The core idea is that it's good for someone or something to go through old
> issues periodically and clean up anything that's no longer relevant, since
> having a bunch of irrelevant issues lying around is poor project hygiene.
> No human is really volunteering for this, hence the bot. The fact that it
> bumps things before closing them is useful too, since it sort of forces
> periodic re-consideration of relevancy.
>
> > > The effect should be giving us an
> > > open issues list that more accurately respects the issues that people
> in
> > > the community feel are important.
> > >
> > The list would still be too long to be comprehensible or digestible for
> > anybody, nor that anyone is expected to go through the full list at any
> > time.
>
> Maybe so, but I would really hope that with a shorter list, it could
> potentially be more digestible. At least wouldn't have a large amount of
> irrelevant issues. If you look through our older issues, so many of them
> are irrelevant or questionably relevant to today's Druid. This is fine if
> nobody ever looks at them, which is probably the case for regular
> contributors, who I assume mostly interact with issues through
> notifications. But it can be misleading to those that don't pay attention
> to the project every day.
>
> > Personally, I open many issues
> > which I don't really plan to work on in any foreseeable future, just to
> > record my ideas and thoughts so that they can be discovered by other
> > developers (and myself) later, and referenced to from future discussions,
> > issues, and PRs. I see a real practical value in it, as I routinely link
> to
> > my own old issues (and re-read them, refreshing my old thoughts on the
> > topic) in Druid development. I don't want to take on a burden of
> regularly
> > repel the stalebot from all of these issues.
>
> This is a tough one. I did think about it and there are ups and downs. The
> upside of stalebot in this case is that these 'idea and thoughts' issues
> can become irrelevant over time (the underlying area of code has been
> refactored and nobody updated the issue, etc) and so it's good to close
> issues that may no longer be relevant. The downside is that the 'idea and
> thoughts' issues tend to naturally be dormant for a long time, and the
> stalebot can be annoying. There is a label "Evergreen" that can be used to
> ward off the stalebot (it will ignore anything with that label) that can be
> used to solve the latter problem. It's probably not good to have a ton of
> issues labeled this way, since they can become irrelevant over time, but it
>

Separation of dev and issue streams into substreams.

[Roman Leventov]

I've stumbled upon a Linus Torvalds' message regarding issue tracking that
resonated with me (https://yarchive.net/comp/linux/bug_tracking.html):

> The thing is, bugzilla is totally broken because it's designed to help
track bugs, but it's *not* designed to actually handle the much harder
problem, which is to actually get the *right* developers to be aware of the
*right* bugs!
>
> And both of those "right"s are important. Spamming everybody will just
mean that everybody tunes them out. And spamming even the right developers
with useless bugreports will also just cause them to tune out the good ones.

Disclaimer: I gave up tracking all Druid issues about half a year ago,
there is now too much noise for me there. I very occasionally visit my
notifications on Github. I'm probably not alone.

Please don't assume that just publishing PR is enough to let everybody know
about it. @-mention people whose opinion you think may be important for an
issue or a PR. Send a message to the dev mailing list for important
issues/PRs.

In general, it would be probably helpful to separate mailing lists for
high-level things in Druid: ingestion/Kafka, querying, security/auth,
approximate/sketches/complex aggregators stuff, SQL, coordination/segment
balancing/ZK/discovery and setup bots that copy the right issues and PRs to
the right lists based on tags.

Re: Stalebot for issues

[Roman Leventov]

> To me it makes sense to close even "Feature" ideas that have no
> constituency, since it is just clutter to have a bunch of feature ideas
> around that nobody is actively pushing.

I have experience as a user (feature asker) of projects which adopt this
policy and it always feels bad to me when my issue is closed "due to lack
of activity". What activity do they expect? I'm not a developer of this
project so, realistically, I cannot contribute to it. However, the problem
is real and it causes real pain when I use the product (project, library,
etc). So it always feels to me that the developers just want to feel
comfortable (as described in the stalebot's documentation cited above in
this thread) and see a small number of open issues at the expense of
alienating users to some little extent. So, IMO, it's better to fix our
perception instead about a large and ever-growing number of issues.

> "Performance" and "Refactoring" makes more sense to consider evergreen

Then "Improvement" should be there, too ("Performance" and "Refactoring"
are just special cases of "Improvement"), as well as regular "Area - "
tags, because "Improvement" is often omitted: generic "improvement" is the
default intention of an issue unless tagged to something different (such as
"bug").

> Without that, some perfectly good ideas might be totally forgotten, open
forever but never looked at. I'm ok either way on these two labels, I
suppose.

Perhaps issue priorities is a better tool for tackling this rather than
regular notification of just the author of the issue. Tags give visibility
for other developers and provide a way to browse the pool of impactful
ideas. Priorities used to be used in the past but then people stopped using
them. The only problem with priorities that I see is that they are
subjective. "Impact/effort ratio" is something more objective.

On Tue, 25 Jun 2019 at 21:07, Julian Hyde  wrote:

> I claim that features have a different lifecycle to bugs. There may not be
> a strong case for doing a particular feature today, but in a year, there
> may be a greater demand. If a bugs are not fixed, their importance usually
> declines over time.
>
> Are people able to vote for features in GitHub issues? Are they able to
> vote to them if they are closed? I think it’s useful for people to continue
> to chime in on features, and eventually build consensus about what should
> be built.
>
> Perhaps a label “not on roadmap” on a feature is all that is necessary to
> manage people’s expectations. I agree that closing bugs makes sense because
> (for some reason!) users assume that developers intend to fix every single
> bug.
>
> Julian
>
>
>
> > On Jun 25, 2019, at 8:55 AM, Gian Merlino  wrote:
> >
> > To me it makes sense to close even "Feature" ideas that have no
> > constituency, since it is just clutter to have a bunch of feature ideas
> > around that nobody is actively pushing. However this starts to remind me
> of
> > the Wikipedia "deletionism vs. inclusionism" debate:
> > https://en.wikipedia.org/wiki/Deletionism_and_inclusionism_in_Wikipedia
> which
> > simmers even to this day.
> >
> > "Performance" and "Refactoring" makes more sense to consider evergreen,
> > although there may still be some benefit in stalebotting them anyway,
> since
> > the bot bumps things periodically to encourage reconsideration. Without
> > that, some perfectly good ideas might be totally forgotten, open forever
> > but never looked at. I'm ok either way on these two labels, I suppose.
> >
> >
> > On Mon, Jun 24, 2019 at 11:36 AM Roman Leventov 
> > wrote:
> >
> >> I wrote previous messages in this thread before I've discovered that the
> >> stalebot send me more than 100 messages. (That shouldn't be surprising
> >> since I'm the author of 174 open issues in Druid:
> >>
> >>
> https://github.com/apache/incubator-druid/search?p=1&q=is%3Aopen+author%3Aleventov+is%3Aissue&type=Issues
> >> ).
> >> As an experiment, I'll try to go over all notifications and post here
> how
> >> many of them can actually be closed now.
> >>
> >> On the other hand, I've realized that a big and a legitimate case for
> >> stalebot closing issues are the issues of "Problem report" kind (
> >>
> >>
> https://github.com/apache/incubator-druid/issues/new?assignees=&template=problem_report.md&title=
> >> ).
> >> The reasoning is that
> >> - As time passes, the issue may be fixed in the newer D

Re: Code review

[Roman Leventov]

I've created the following PR based on this discussion and the document
that I've started to prepare last year:
https://docs.google.com/document/d/17EEKT6fih9Dd5NfXjBoECcKbVp1eOB2vb3jKqTF9pPc/edit
.

  https://github.com/apache/incubator-druid/pull/7991

I believe having agreed upon development principles will be a strong
foundation for future community collaboration. Please review.

On Mon, 13 May 2019 at 18:29, Roman Leventov  wrote:

> A quote from Steve Tockey regarding documentation and comments and the
> cost of maintaining software:
> https://medium.com/@leventov/steve-tockey-on-software-documentation-and-comments-470751227a63
>
> On Mon, 29 Apr 2019 at 19:30, Fangjin Yang  wrote:
>
>> Strong +1 agreement with Gian. I definitely see there are anal code
>> reviews
>> focused too much on trying to force changes that make no difference to the
>> core functionality being contributed, and are instead a reflection of
>> personal biases of the reviewer. These types of low quality code reviews
>> provide absolutely no value to the Druid community, and instead serve to
>> damage the community engagement, and slow down development.
>>
>> On Thu, Apr 25, 2019 at 11:48 AM Julian Hyde  wrote:
>>
>> > +1
>> >
>> > As someone who reviews Druid release candidates but does not develop
>> Druid
>> > code, I wish that “mvn test” would “just work”. I can imagine that
>> > first-time and occasional contributors are in a similar boat to me.
>> >
>> > I know this is not an easy thing to achieve, because the test suite also
>> > has to work for more experienced contributors.
>> >
>> > Julian
>> >
>> >
>> > > On Apr 25, 2019, at 11:42 AM, David Glasser <
>> glas...@apollographql.com>
>> > wrote:
>> > >
>> > > As a new contributor, I've actually really appreciated the careful and
>> > high
>> > > quality code reviews I've received (primarily from Jihoon).
>> > >
>> > > The real source of friction for me has not been the feedback from
>> > > developers, but the painfulness of running the tests.
>> > >
>> > > - Figuring out how to locally run the full suite of tests that CI will
>> > run
>> > > is not super documented.
>> > > - Understanding how to just run the tests that are most relevant to
>> your
>> > > change (either from CLI or IntelliJ) isn't well documented.  It's
>> > > especially unclear what things can be perfectly successfully run from
>> > > IntelliJ's own test runners and what things really require you to run
>> > them
>> > > through Maven.  (Druid is also the first time I've used Maven directly
>> > and
>> > > it's a huge challenge for me to understand what's going on and how to
>> run
>> > > it properly.)
>> > > - Many of the tests are just very slow and maybe could be parallelized
>> > > better?
>> > >
>> > > It's enough of a pain that I often when iterating wouldn't even bother
>> > > trying to run tests manually but would just push to GitHub and let
>> Travis
>> > > run it instead. But Travis itself is very slow and while it's somewhat
>> > > parallelized, it's not super parallelized — and as a lowly PR author I
>> > > can't even kill the overall job if I push a new version of the PR or
>> if a
>> > > sub-job already failed.  So this would frequently add "round trip"
>> times
>> > of
>> > > half an hour or more in the middle of trying to take the good advice
>> from
>> > > reviewers to heart.
>> > >
>> > > So while I agree that it would be great to move as many of the checks
>> as
>> > > possible into CI from "core dev teams mind", I'd also encourage folks
>> > with
>> > > more time and expertise than I have now to put effort into making the
>> CI
>> > > experience faster and easier.  (I wonder if some of the issues could
>> just
>> > > be solved with money, eg running on more powerful Travis machines or
>> > > parallelizing the slower tests even further onto a larger number of
>> > > containers.  I've also generally found in my own work that CircleCI at
>> > > least feels faster and easier to work with than Travis FWIW.)
>> > >
>> > > --dave
>> > >
>> > > On Mon, Apr 22, 2019 at 7:44 PM Gian Merlino  wrot

Re: Stalebot for issues

[Roman Leventov]

I've proposed to add more exempt labels and set the closing timeout to 28
days here: https://github.com/apache/incubator-druid/pull/8084.

On Sat, 6 Jul 2019 at 01:35, Gian Merlino  wrote:

> You raise a good point but I don't think leaving issues open with no
> response forever is a good solution either. That's probably what would have
> happened to your issues if we didn't have a stalebot. The ideal thing is to
> strive to respond to every reported issue, which hopefully we can pull
> together as a community to do.
>
> On Fri, Jul 5, 2019 at 3:22 PM Prashant Deva 
> wrote:
>
> > i agree with you, but do consider the following case:
> >
> > I am new to druid. I report the above 2 bugs. They don’t get a response.
> > Then a bot closes them automatically.
> > As a new user, I may then not be motivated to report further bugs.
> >
> >
> >
> > On Thu, Jul 4, 2019 at 9:13 PM Gian Merlino  wrote:
> >
> > > I think that would be a perfect reason to comment on those issues and
> > > mention that they are still relevant. The stalebot message even invites
> > you
> > > to do so. IMO, one of the services provided by the stalebot is to
> remind
> > > people to take a look at older issues and check if they are still
> > relevant,
> > > otherwise they would be likely to sit open forever with nobody
> reviewing
> > > them.
> > >
> > > On Thu, Jul 4, 2019 at 9:06 PM Prashant Deva 
> > > wrote:
> > >
> > > > stalebot just closed my issues 7473 and 7521.
> > > >
> > > > Both bugs are still present.
> > > >
> > > > they were closed because the bug reports themselves didn’t receive a
> > > reply.
> > > >
> > > > Not receiving a reply did not make the bugs go away. Yet due to
> > stalebot,
> > > > the bugs are now closed.
> > > >
> > > > On Wed, Jun 26, 2019 at 10:28 AM Roman Leventov <
> leventov...@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > > To me it makes sense to close even "Feature" ideas that have no
> > > > > > constituency, since it is just clutter to have a bunch of feature
> > > ideas
> > > > > > around that nobody is actively pushing.
> > > > >
> > > > > I have experience as a user (feature asker) of projects which adopt
> > > this
> > > > > policy and it always feels bad to me when my issue is closed "due
> to
> > > lack
> > > > > of activity". What activity do they expect? I'm not a developer of
> > this
> > > > > project so, realistically, I cannot contribute to it. However, the
> > > > problem
> > > > > is real and it causes real pain when I use the product (project,
> > > library,
> > > > > etc). So it always feels to me that the developers just want to
> feel
> > > > > comfortable (as described in the stalebot's documentation cited
> above
> > > in
> > > > > this thread) and see a small number of open issues at the expense
> of
> > > > > alienating users to some little extent. So, IMO, it's better to fix
> > our
> > > > > perception instead about a large and ever-growing number of issues.
> > > > >
> > > > > > "Performance" and "Refactoring" makes more sense to consider
> > > evergreen
> > > > >
> > > > > Then "Improvement" should be there, too ("Performance" and
> > > "Refactoring"
> > > > > are just special cases of "Improvement"), as well as regular "Area
> -
> > "
> > > > > tags, because "Improvement" is often omitted: generic "improvement"
> > is
> > > > the
> > > > > default intention of an issue unless tagged to something different
> > > (such
> > > > as
> > > > > "bug").
> > > > >
> > > > > > Without that, some perfectly good ideas might be totally
> forgotten,
> > > > open
> > > > > forever but never looked at. I'm ok either way on these two
> labels, I
> > > > > suppose.
> > > > >
> > > > > Perhaps issue priorities is a better tool for tackling this rather
> > than
> > > > > regular notification of just the author of the issue. Tags give
> > > > v

Renaming 'bug' label to 'defect'

[Roman Leventov]

There have been many times, and several in the last few days (
https://github.com/apache/incubator-druid/issues/8291,
https://github.com/apache/incubator-druid/issues/8263) when I wanted to
label an issue as 'bug' but the semantics of the word "bug" don't really
apply. For me, "bug" means "somebody made a programming mistake at some
point". This doesn't apply to the issues linked above. I would say about
them "the system, as it is currently implemented, fails (or may fail) under
certain circumstances and shall be improved". PRs which would fix these
issues shall be labelled 'Improvement'.

I think renaming 'bug' into 'defect' would be useful to broaden the
applicability of the label.

Re: Renaming 'bug' label to 'defect'

[Roman Leventov]

Then the definition should be even wider: there are many situations when
there is only potential for user-visible bad behavior. The classic
situation when a bug is masked by another bug. The fact that the first bug
is not exposed (yet) is not a reason to not call it a bug.

Also, there are internal bugs - leading to faulty intra-cluster behavior
which may be mitigated by some cluster's redundancy mechanisms. Like a bug
in the ZK configuration which leads to frequent reconnects, which doesn't,
however, lead to anything visible by a Druid user nor operator (note: the
situation is made up). This could have been called "improvement" unless it
was caused by a programming mistake. I would not call anything which is
caused by a programming mistake a mere "improvement", it's always a "bug"
for me.

On Thu, 15 Aug 2019 at 08:08, Gian Merlino  wrote:

> I think it'd be ok to call those examples bugs. In professional programming
> contexts I've always used 'bug' in a wide sense, meaning any sort of flaw
> or issue in a system that causes user-visible bad behavior. It could be a
> programming mistake, a design flaw, or even a problem with a dependency.
> (Conversely, if there's no user-visible bad behavior, I wouldn't call it a
> bug; maybe it'd be an improvement.)
>
> On Tue, Aug 13, 2019 at 6:51 AM Roman Leventov 
> wrote:
>
> > There have been many times, and several in the last few days (
> > https://github.com/apache/incubator-druid/issues/8291,
> > https://github.com/apache/incubator-druid/issues/8263) when I wanted to
> > label an issue as 'bug' but the semantics of the word "bug" don't really
> > apply. For me, "bug" means "somebody made a programming mistake at some
> > point". This doesn't apply to the issues linked above. I would say about
> > them "the system, as it is currently implemented, fails (or may fail)
> under
> > certain circumstances and shall be improved". PRs which would fix these
> > issues shall be labelled 'Improvement'.
> >
> > I think renaming 'bug' into 'defect' would be useful to broaden the
> > applicability of the label.
> >
>

Re: A list of issues for new committers

[Roman Leventov]

I don't use "Contributions Welcome" exclusively for starter issues. I do
_not_ put this label though on issues that are impossible or hard to
validate without a production cluster (with specific type of load,
perhaps). So the assumption is that anyone just with a laptop and IDE could
contribute these issues, in theory.

On Fri, 4 Oct 2019, 02:59 Vadim Ogievetsky,  wrote:

> Hi all,
>
> I would like to organize a set of issues (via a label) that are good
> starting points for someone wanting to contribute to Druid.
> I want to have the link to the issues filtered on that label to be part of
> the README.
> This would encourage people new to Druid to contribute and get involved in
> the community.
>
> Right now there are two labels that sort of look like they might be
> talking about something relevant:
>
> "Contributions Welcome" (
> https://github.com/apache/incubator-druid/labels/Contributions%20Welcome)
>
>   This label seems to only be used by Leventov. I am not a Java dev so it
> is hard for me to judge if these issues are really good starter issues.
> Maybe this is more of a "Help Wanted" label.
>
> "Difficulty - Easy" (
> https://github.com/apache/incubator-druid/labels/Difficulty%20-%20Easy)
>
>   This label seems more aligned with what I want but I find its text too
> off-putting to use.
>   "Easy"? for who? It would not be "easy" for me to to fix a distributed
> systems bug no matter how small.
>   On the flip side the console might have a CSS bug that is "easy" in my
> eyes but will make a Java dev very sad.
>   While I am on this subject I think these "Difficulty - *" labels are
> generally unhelpful - does anyone actually use them?
>
> My suggestion is as follows:
>
> 1. Establish a "Starter" label, link it from the README
>
> 2. Rename the "Difficulty - Easy" to "Starter" and review to see if some
> issues could be added or removed
>
> 3. Remove all the "Difficulty - *" labels
>
>
> I would love to get some feedback on this.
>
> Best regards,
> Vadim
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> For additional commands, e-mail: dev-h...@druid.apache.org
>
>

Looking out of the Druid's development bubble at modern Java testing practices

[Roman Leventov]

A few days ago I've stumbled upon a blog post "Modern Best Practices for
Testing in Java" (
https://phauer.com/2019/modern-best-practices-testing-java/) and learned a
lot of new things from it.

In a lot of cases, how testing is done in Druid goes against these best
practices:
 - "Given, When, Then" structure: in Druid, rarely followed or clarified
via blank lines.
 - Use the Prefixes “actual*” and “expected*: not in Druid, usually it's
something1 / something2 and what is expected and what is actual is unclear
(and sometimes confused in assert() args)
 - Heavily Use Helper Functions: used sometimes in Druid, but by far not
enough,  in my perception
 - Don’t Extend Existing Tests To “Just Test One More Tiny Thing: not
everywhere, but in a lot of places this practice is abused in Druid tests
 - Insert Test Data Right In The Test Method:  test data is often inserted
in @Before in Druid
 - Favor Composition Over Inheritance: there are some hierarchies in Druid
 - Don’t Write Too Much Logic: Druid tests feel like *all*
boilerplate/noise/scaffolding logic
 - Don’t Use In-Memory Databases For Tests: Druid does toy database Derby
for tests :)
 - Use AssertJ: Druid doesn't use AssertJ (I've just added AssertJ to
dependencies in https://github.com/apache/incubator-druid/pull/8564 in one
module)
 - Use JUnit5: ¯\_(ツ)_/¯
 - Mock Remote Service: in Druid, we don't use OkHttp's MockWebServer
 - Use Awaitility for Asserting Asynchronous Code: in Druid, we use ad-hoc
while loops with sleep(10), sleep(100), etc.
 - Separate Asynchronous Execution and Actual Logic: I think, this is one
of the most important points. In Druid, in almost all cases, execution
management (thread pools), dependency injection (and dependencies in
general), and the "core" logic are alloyed in a single class. This makes
testing the core logic very painful with a lot of mocking and setup
ceremony, breaking frequently. Following the Humble Object pattern (
http://xunitpatterns.com/Humble%20Object.html) would make writing *and
supporting* much less laborious.

These things cannot be fixed in one day because test code has a lot of
inertia, but I believe to improve developer productivity all active Druid
developers should start using most of these practices in all new tests
being written, demand other contributors align their tests with these
practices in code reviews, and take some time to think about strategic ways
to refactor a bulk of existing tests with little effort
(semi-automatically) that can at the same time improve their quality even
by a little.

In my experience, wrestling with tests is a huge (often the biggest) part
of changing existing code in Druid, so we pay a huge productivity tax by
not paying enough attention to the quality of the test codebase.

Re: Looking out of the Druid's development bubble at modern Java testing practices

[Roman Leventov]

Nothing in particular. I think just reading the article. And also actually
paying attention to test quality. In the past, on many occasions, I've
skipped reading the test code in pull-requests completely or glanced over
it very cursory which is a bad practice.

The questions that developers and reviewers should ask themselves are
 - Will it be easy to add more tests to this class?
 - Will it be easy to support existing and new tests?
 - Will the test break (or would require rework) when somebody changes
implementation internals, e. g. due to mocking not working properly?

On Fri, 4 Oct 2019 at 21:57, Jad Naous  wrote:

> That's a great link, thank you! What do you think we should change in the
> process to start following these patterns? Should the committers be more
> strict on what gets committed?
>
> On Fri, Oct 4, 2019 at 9:44 AM Roman Leventov  wrote:
>
> > A few days ago I've stumbled upon a blog post "Modern Best Practices for
> > Testing in Java" (
> > https://phauer.com/2019/modern-best-practices-testing-java/) and
> learned a
> > lot of new things from it.
> >
> > In a lot of cases, how testing is done in Druid goes against these best
> > practices:
> >  - "Given, When, Then" structure: in Druid, rarely followed or clarified
> > via blank lines.
> >  - Use the Prefixes “actual*” and “expected*: not in Druid, usually it's
> > something1 / something2 and what is expected and what is actual is
> unclear
> > (and sometimes confused in assert() args)
> >  - Heavily Use Helper Functions: used sometimes in Druid, but by far not
> > enough,  in my perception
> >  - Don’t Extend Existing Tests To “Just Test One More Tiny Thing: not
> > everywhere, but in a lot of places this practice is abused in Druid tests
> >  - Insert Test Data Right In The Test Method:  test data is often
> inserted
> > in @Before in Druid
> >  - Favor Composition Over Inheritance: there are some hierarchies in
> Druid
> >  - Don’t Write Too Much Logic: Druid tests feel like *all*
> > boilerplate/noise/scaffolding logic
> >  - Don’t Use In-Memory Databases For Tests: Druid does toy database Derby
> > for tests :)
> >  - Use AssertJ: Druid doesn't use AssertJ (I've just added AssertJ to
> > dependencies in https://github.com/apache/incubator-druid/pull/8564 in
> one
> > module)
> >  - Use JUnit5: ¯\_(ツ)_/¯
> >  - Mock Remote Service: in Druid, we don't use OkHttp's MockWebServer
> >  - Use Awaitility for Asserting Asynchronous Code: in Druid, we use
> ad-hoc
> > while loops with sleep(10), sleep(100), etc.
> >  - Separate Asynchronous Execution and Actual Logic: I think, this is one
> > of the most important points. In Druid, in almost all cases, execution
> > management (thread pools), dependency injection (and dependencies in
> > general), and the "core" logic are alloyed in a single class. This makes
> > testing the core logic very painful with a lot of mocking and setup
> > ceremony, breaking frequently. Following the Humble Object pattern (
> > http://xunitpatterns.com/Humble%20Object.html) would make writing *and
> > supporting* much less laborious.
> >
> > These things cannot be fixed in one day because test code has a lot of
> > inertia, but I believe to improve developer productivity all active Druid
> > developers should start using most of these practices in all new tests
> > being written, demand other contributors align their tests with these
> > practices in code reviews, and take some time to think about strategic
> ways
> > to refactor a bulk of existing tests with little effort
> > (semi-automatically) that can at the same time improve their quality even
> > by a little.
> >
> > In my experience, wrestling with tests is a huge (often the biggest) part
> > of changing existing code in Druid, so we pay a huge productivity tax by
> > not paying enough attention to the quality of the test codebase.
> >
>
>
> --
> Jad Naous
> Imply | VP R&D
> 650-521-3425
> jad.na...@imply.io
>  @jadtnaous <https://twitter.com/jadtnaous>
>

Re: Nulls vs Optional

[Roman Leventov]

A few notes:

 (1) There is a very long-running project towards fixing static nullability
problems (identified by IntelliJ). The way of fixing is always properly
using/not using @Nullable and annotating all packages in the project
with @EverythingIsNonNullByDefault (see
https://github.com/apache/incubator-druid/pull/5957#discussion_r206792747,
https://github.com/apache/incubator-druid/pull/8198). I don't believe this
project will ever complete. It would be nice if we were able to have
ratcheting (
https://robertgreiner.com/continuous-code-improvement-using-ratcheting/)
for the number of "Nullability problems" inspection violations in the
project in our TeamCity build, but unfortunately, this is not currently
possible, too.

 (2) However, I think (1) is more a purism than a stressing practical need,
because I don't actually see NPE being a huge source of bugs in production.
NPEs frequently bubble up in tests when you refactor code, but then you fix
tests before committing the code. This is a slightly longer feedback loop
than if these nullability problems didn't allow you to compile the code,
but not dramatically.

 (3) With regard to nullability, it might be a better idea to make Druid a
mixed Java/Kotlin project and write non-performance critical parts of code
and tests in Kotlin. Kotlin has a very succinct syntax for handling
nullability, among other benefits.


On Fri, 11 Oct 2019 at 03:46, Gian Merlino  wrote:

> For reference, a (brief) earlier conversation about this:
> https://github.com/apache/incubator-druid/issues/4275, which links to
> https://github.com/apache/incubator-druid/pull/4254#discussion_r116628607,
> which links to
>
> https://stackoverflow.com/questions/26327957/should-java-8-getters-return-optional-type/26328555#26328555
> .
>
> I really enjoyed programming with Scala's Option type, back when I had
> spend a couple of years writing Scala code. They are awesome. Java
> Optionals are sadly not quite as good, especially in ecosystem support
> (they're not adopted very well in libraries or in the jdk itself) but also
> in functionality (in Scala they're tightly integrated into the collections
> library, which allows for some nice idioms).
>
> After that Scala break, when I came back to Java I wrote a lot of the
> indexing service module. You can tell because it has Guava Optionals
> everywhere. I mostly used it the way that Goetz recommended, as a method
> return type in commonly used interfaces or utility methods. It was a bit
> clunky but it was overall nice. I don't regret it. But now I mostly don't
> use them anymore, and started using null-returns (with @Nullable
> annotations) instead since it jives better with the rest of the codebase.
>
> Jad, or anyone else, have you worked on Java codebases where Optional was
> heavily used? What was the experience like?
>
> Also, has anyone had experience introducing a preference for Optionals into
> a large pre-existing codebase?
>
> On Wed, Oct 9, 2019 at 12:46 PM Jad Naous  wrote:
>
> > Sir Tony Hoare on inventing null while working on ALGOL (from wikipedia
> > below):
> >
> > Speaking at a software conference called QCon London
> >  in 2009, he apologised for inventing the null
> > reference :[23]
> > 
> >
> > I call it my billion-dollar mistake. It was the invention of the null
> > reference in 1965. At that time, I was designing the first comprehensive
> > type system for references in an object oriented language (ALGOL W
> > ). My goal was to ensure that all
> > use of references should be absolutely safe, with checking performed
> > automatically by the compiler. But I couldn't resist the temptation to
> put
> > in a null reference, simply because it was so easy to implement. This has
> > led to innumerable errors, vulnerabilities, and system crashes, which
> have
> > probably caused a billion dollars of pain and damage in the last forty
> > years.
> >
> > How about we stop passing nulls around as method arguments, field values,
> > return values, etc and use Optional instead? Benefits:
> > - No more NPEs
> > - Better documentation through code
> > - Less mistakes
> >
> > I'm not suggesting we go rewrite everything, but rather just starting to
> > only return and accept Optionals in methods/constructors/etc.
> >
> > Jad.
> >
> > --
> > Jad Naous
> > Imply | VP R&D
> > 650-521-3425
> > jad.na...@imply.io
> >
>

Re: A list of issues for new committers

[Roman Leventov]

I  think "Idea" is too vague. I think it's OK to re-label most
"Contributions Welcome" to "Starter", but "Contributions Welcome" could
remain.

On Fri, 11 Oct 2019 at 03:46, Vadim Ogievetsky 
wrote:

> Ok I will remove the Difficulty labels and repurpose "Easy" to "Starter"
> unless anyone objects within 24h.
>
> On 2019/10/09 06:30:06, Gian Merlino  wrote:
> > That is definitely a good property for starter issues (not needing a
> > production cluster to validate).
> >
> > On Fri, Oct 4, 2019 at 12:01 AM Roman Leventov 
> > wrote:
> >
> > > I don't use "Contributions Welcome" exclusively for starter issues. I
> do
> > > _not_ put this label though on issues that are impossible or hard to
> > > validate without a production cluster (with specific type of load,
> > > perhaps). So the assumption is that anyone just with a laptop and IDE
> could
> > > contribute these issues, in theory.
> > >
> > > On Fri, 4 Oct 2019, 02:59 Vadim Ogievetsky, 
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I would like to organize a set of issues (via a label) that are good
> > > > starting points for someone wanting to contribute to Druid.
> > > > I want to have the link to the issues filtered on that label to be
> part
> > > of
> > > > the README.
> > > > This would encourage people new to Druid to contribute and get
> involved
> > > in
> > > > the community.
> > > >
> > > > Right now there are two labels that sort of look like they might be
> > > > talking about something relevant:
> > > >
> > > > "Contributions Welcome" (
> > > >
> https://github.com/apache/incubator-druid/labels/Contributions%20Welcome
> > > )
> > > >
> > > >   This label seems to only be used by Leventov. I am not a Java dev
> so it
> > > > is hard for me to judge if these issues are really good starter
> issues.
> > > > Maybe this is more of a "Help Wanted" label.
> > > >
> > > > "Difficulty - Easy" (
> > > >
> https://github.com/apache/incubator-druid/labels/Difficulty%20-%20Easy)
> > > >
> > > >   This label seems more aligned with what I want but I find its text
> too
> > > > off-putting to use.
> > > >   "Easy"? for who? It would not be "easy" for me to to fix a
> distributed
> > > > systems bug no matter how small.
> > > >   On the flip side the console might have a CSS bug that is "easy"
> in my
> > > > eyes but will make a Java dev very sad.
> > > >   While I am on this subject I think these "Difficulty - *" labels
> are
> > > > generally unhelpful - does anyone actually use them?
> > > >
> > > > My suggestion is as follows:
> > > >
> > > > 1. Establish a "Starter" label, link it from the README
> > > >
> > > > 2. Rename the "Difficulty - Easy" to "Starter" and review to see if
> some
> > > > issues could be added or removed
> > > >
> > > > 3. Remove all the "Difficulty - *" labels
> > > >
> > > >
> > > > I would love to get some feedback on this.
> > > >
> > > > Best regards,
> > > > Vadim
> > > >
> > > > -
> > > > To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> > > > For additional commands, e-mail: dev-h...@druid.apache.org
> > > >
> > > >
> > >
> >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org
> For additional commands, e-mail: dev-h...@druid.apache.org
>
>

Re: Nulls vs Optional

[Roman Leventov]

Not to say that it's impossible to write exactly as fast code in Kotlin as
in Java (or faster - thanks to its inline functions:
https://kotlinlang.org/docs/reference/inline-functions.html), there are a
few reasons why it requires greater discipline to do so in Kotlin than in
Java:
 1) It's collection types List, Map are natural to use
and too easy to forget to use fastutil's primitive collections. However,
this is currently an issue in Java code, too: see
https://github.com/apache/incubator-druid/issues/6861.
 2) Collection transformations are eager on each step by default, so you
have not to forget to insert `.asSequence()` in the beginning of a
multi-step transformation to avoid creating excessive garbage
 3) companion object's methods are actually instance methods (not statics),
so you have not to forget to add @JvmStatic annotation.

On Sat, 12 Oct 2019 at 09:43, Julian Hyde  wrote:

> I'm glad you brought up Kotlin. Its approach to nullable types is
> elegant and pragmatic. If a string argument allows nulls, you give it
> type 'String?'; if not, give it type 'String'[1]. Unlike Scala's
> Option or Java's Optional, the arguments have the same runtime type;
> the difference between 'String?' and 'String' is handled by the
> compiler and has no runtime overhead.
>
> As far as I can tell, Kotlin is as efficient as Java (since most of
> its features are syntactic sugar around Java features, and you can
> avoid extensions like delegates and receivers). There's no reason to
> only "write non-performance critical parts of code and tests in
> Kotlin".
>
> There are, however, plenty of other good reasons to keep Druid in good
> old Java. Better the devil you know.
>
> Julian
>
> [1] https://kotlinlang.org/docs/reference/null-safety.html
>
> On Fri, Oct 11, 2019 at 5:16 AM Roman Leventov 
> wrote:
> >
> > A few notes:
> >
> >  (1) There is a very long-running project towards fixing static
> nullability
> > problems (identified by IntelliJ). The way of fixing is always properly
> > using/not using @Nullable and annotating all packages in the project
> > with @EverythingIsNonNullByDefault (see
> >
> https://github.com/apache/incubator-druid/pull/5957#discussion_r206792747,
> > https://github.com/apache/incubator-druid/pull/8198). I don't believe
> this
> > project will ever complete. It would be nice if we were able to have
> > ratcheting (
> > https://robertgreiner.com/continuous-code-improvement-using-ratcheting/)
> > for the number of "Nullability problems" inspection violations in the
> > project in our TeamCity build, but unfortunately, this is not currently
> > possible, too.
> >
> >  (2) However, I think (1) is more a purism than a stressing practical
> need,
> > because I don't actually see NPE being a huge source of bugs in
> production.
> > NPEs frequently bubble up in tests when you refactor code, but then you
> fix
> > tests before committing the code. This is a slightly longer feedback loop
> > than if these nullability problems didn't allow you to compile the code,
> > but not dramatically.
> >
> >  (3) With regard to nullability, it might be a better idea to make Druid
> a
> > mixed Java/Kotlin project and write non-performance critical parts of
> code
> > and tests in Kotlin. Kotlin has a very succinct syntax for handling
> > nullability, among other benefits.
> >
> >
> > On Fri, 11 Oct 2019 at 03:46, Gian Merlino  wrote:
> >
> > > For reference, a (brief) earlier conversation about this:
> > > https://github.com/apache/incubator-druid/issues/4275, which links to
> > >
> https://github.com/apache/incubator-druid/pull/4254#discussion_r116628607,
> > > which links to
> > >
> > >
> https://stackoverflow.com/questions/26327957/should-java-8-getters-return-optional-type/26328555#26328555
> > > .
> > >
> > > I really enjoyed programming with Scala's Option type, back when I had
> > > spend a couple of years writing Scala code. They are awesome. Java
> > > Optionals are sadly not quite as good, especially in ecosystem support
> > > (they're not adopted very well in libraries or in the jdk itself) but
> also
> > > in functionality (in Scala they're tightly integrated into the
> collections
> > > library, which allows for some nice idioms).
> > >
> > > After that Scala break, when I came back to Java I wrote a lot of the
> > > indexing service module. You can tell because it has Guava Optionals
> > > everywhere. I mostly used it th

To the note of committers: please re-label "Uncategorized problem report" issues

[Roman Leventov]

Please re-label "Uncategorized problem report" issues when you identify the
nature of the issue (bug, work as intended, misconfiguration, misuse,
etc.). Example: https://github.com/apache/incubator-druid/issues/8667.

Allow static imports in tests

[Roman Leventov]

It seems to me that Druid's policy to prohibit static imports makes more
harm than good in unit test code. It discourages the usage of static
fixtures and encourages abstraction/inheritance, as I discussed here:
https://github.com/apache/incubator-druid/pull/8564#issue-319753256, see
"Refactoring of tests" section.

Alternative: allow only static imports from selected classes: Assert,
Assertions, EasyMock, etc. IntelliJ's "Static import" inspection supports
such fine configuration.

Don't auto-close Bug-labelled PRs after a period of inactivity with stale bot.

[Roman Leventov]

I think it would be better to configure the stale-bot to leave a message in
a PR every 60 days, but not actually close PRs with Bug label. Feels a
little like sweeping problems under the carpet.

Re: Allow static imports in tests

[Roman Leventov]

Ok, created issue https://github.com/apache/incubator-druid/issues/8885.

I generally agree about EasyMock, but as of now, it's a fact of life that
it is used a lot in tests.

On Fri, 15 Nov 2019 at 21:36, Gian Merlino  wrote:

> I quite dislike EasyMock (I think it leads to brittle tests that
> over-couple test code with production code). But that comment aside, I
> think it is reasonable to use static imports for DSL-type stuff in tests,
> which it sounds like what you are suggesting. So that sounds good to me. I
> would still suggest generally avoiding them in production code.
>
> On Fri, Nov 8, 2019 at 1:40 AM Roman Leventov  wrote:
>
> > It seems to me that Druid's policy to prohibit static imports makes more
> > harm than good in unit test code. It discourages the usage of static
> > fixtures and encourages abstraction/inheritance, as I discussed here:
> > https://github.com/apache/incubator-druid/pull/8564#issue-319753256, see
> > "Refactoring of tests" section.
> >
> > Alternative: allow only static imports from selected classes: Assert,
> > Assertions, EasyMock, etc. IntelliJ's "Static import" inspection supports
> > such fine configuration.
> >
>

Request for Review: MetadataSegmentManager refactoring

[Roman Leventov]

Could somebody please review this PR:
https://github.com/apache/incubator-druid/pull/7306 (Reconcile terminology
and method naming to 'used/unused segments'; Rename MetadataSegmentManager
to MetadataSegments)? Thanks.

Re: 8399 Migrating Guava to Caffeine

[Roman Leventov]

To avoid lingering of Guava for a few more Druid releases in code, the
"guava" config value could just forcibly use caffeine cache,
concurrencyLevel parameter ignored, and appropriate warning messages
logged. There is no harm in this, Caffeine's concurrency is practically
"elastic" and doesn't demand concurrencyLevel.

On Mon, 6 Jan 2020 at 01:13, Gian Merlino  wrote:

> Hey JJ,
>
> I think your idea of adding a new option and deprecating "guava" is a good
> way forward.
>
> Gian
>
> On Fri, Dec 27, 2019 at 7:50 AM JJ Meyer  wrote:
>
> > Hello all,
> >
> > I'm planning on contributing for the first time. I'm working on
> > https://github.com/apache/druid/issues/8399. No issues seem to occur
> when
> > replacing guava with caffeine in any of the classes posted in the issue
> > with exception of the class, OnHeapLoadingCache.
> >
> > I wanted to post something here as I believe it will require a config
> > change to use caffeine in this case. (
> >
> >
> https://druid.apache.org/docs/latest/development/extensions-core/druid-lookups.html#example-loading-on-heap-guava
> > ).
> > It seems as if guava's `concurrencyLevel` is not a property in caffeine's
> > cache. Currently there are types `guava` and `mapDb`. To prevent a config
> > change a third cache type, caffeine, can be added and the guava cache can
> > be marked as deprecated and potentially removed in some future major
> > release. The configs would be identical to the guava type with exception
> of
> > `concurrencyLevel` (it will be removed for the caffeine option).
> >
> > What do you all think of this? Is there another solution that is
> preferred?
> >
> > Thanks,
> > JJ
> >
>

Druid and machine learning

[Roman Leventov]

Hello Druid developers, what do you think about the future of Druid &
machine learning?

Druid has been great at complex aggregations. Could (should?) It make
inroads into ML? Perhaps aggregators which apply the rows against some
pre-trained model and summarize results.

Should model training stay completely external to Druid, or it could be
incorporated into Druid's data lifecycle on a conceptual level, such as a
recurring "indexing" task which stores the result (the model) in Druid's
deep storage, the model automatically loaded on historical nodes as needed
(just like segments) and certain aggregators pick up the latest model?

Does this make any sense? In what cases Druid & ML will and will not work
well together, and ML should stay a Spark's prerogative?

I would be very interested to hear any thoughts on the topic, vague ideas
and questions.