Re: [VOTE] Apache Kudu 1.6.0 RC1

[David Alves]

+1 built and ran debug/release tests on macso high sierra.

-david

On Tue, Dec 5, 2017 at 7:16 PM, Dan Burkert  wrote:
> +1 built and ran tests on macos.
>
> - Dan
>
> On Tue, Dec 5, 2017 at 7:06 PM, Mike Percy  wrote:
>
>> +1
>>
>> I built 1.6.0-RC1 on RHEL 7 in RELEASE mode.
>>
>> - Checksums and sigs match
>> - README.txt and LICENSE.txt look good
>> - RAT check passed
>> - All tests passed except for one flaky test (delete_table-itest) and one
>> environment-related test issue (on kudu-tool-test), the latter of which I
>> filed as KUDU-2234
>> - Tested the kudu-client Maven repository jar against one of the old
>> kudu-examples example programs
>> - Performed some manual failover testing on a 10-node cluster with flash
>> disks and everything looked solid
>>
>> Mike
>>
>> On Fri, Dec 1, 2017 at 11:10 PM, Mike Percy  wrote:
>>
>> > Hi,
>> >
>> > The Apache Kudu team is happy to announce the first release candidate for
>> > Apache Kudu 1.6.0.
>> >
>> > Apache Kudu 1.6.0 is a minor release that offers many improvements and
>> > fixes since the prior release.
>> >
>> > The is a source-only release. The artifacts have been staged here:
>> > https://dist.apache.org/repos/dist/dev/kudu/1.6.0-RC1/
>> >
>> > Java convenience binaries in the form of a Maven repository are staged
>> > here:
>> > https://repository.apache.org/content/repositories/orgapachekudu-1017
>> >
>> > It is tagged in Git as 1.6.0-RC1 and the corresponding git hash is the
>> > following:
>> > https://git-wip-us.apache.org/repos/asf?p=kudu.git;a=commit;h=
>> > 7e28dfc728e54e82d6cbc11fdcd4b9e62f2dfd66
>> >
>> > The release notes can be found here:
>> > https://github.com/apache/kudu/blob/1.6.0-RC1/docs/release_notes.adoc
>> >
>> > The KEYS file to verify the artifact signatures can be found here:
>> > https://dist.apache.org/repos/dist/release/kudu/KEYS
>> >
>> > I'd suggest going through the README and the release notes, building
>> Kudu,
>> > and running the unit tests. Testing out the Maven repo would also be
>> > appreciated.
>> >
>> > The vote will run until Tuesday, December 5th at 11PM PDT. We'd normally
>> > run a vote for 3 full days but since we're headed into the weekend now
>> > let's do 4 days instead.
>> >
>> > Thanks,
>> > Mike
>> >
>> >
>>

Re: Adding extra data to Kudu::Status

[David Alves]

I'd be pro having an _optional_ PB or some other optional way to keep extra
metadata in a status.
In most cases (no extra metadata) this wouldn't have an additional cost
when we're copying statuses around and the PB would have the obvious
advantage of being easy to send to clients, when we need more error
information to handle things properly.
I don't think extra args are going to be scalable since we'd need to
basically pipe them everywhere we could get an EIO.
Regarding the templated option, how would the caller know that it can be
downcast? Is it always in the same type of error? Would you need rtti?

-david

On Mon, Apr 24, 2017 at 12:46 PM, Alexey Serbin 
wrote:

> Hi Andrew,
>
> The instances of Status objects are copied around very often, so putting
> something heap-allocated into the Status class would require dealing with
> that (wrapping those into shared_ptr? But I'm not sure it's worth it).
>
> As for down-casting: would it be possible in your case to limit the
> exchange of ExtendedStatus instances only to some particular methods and
> make sure your methods always return the ExternedStatus?  That would allow
> to avoid downcasting.
>
> I considered a similar approach while propagating RPC error codes along
> with Status in the code around connection negotiation, but eventually I
> decided to go with an additional output parameter for a couple of methods.
>
>
> Best regards,
>
> Alexey
>
>
> On 4/24/17 12:33 PM, Andrew Wong wrote:
>
>> Hi everyone,
>>
>> In my work on disk failure handling, it would be very useful to pass extra
>> information along with Statuses. To add some context, I would like to
>> propagate the uuid of a failed disk to the high-level operations that may
>> have caused the failure (e.g. FlushDeltaMemStoresOp.Perform(), etc.),
>> which
>> can then error-handle based on this uuid (e.g. tell the FsManager,
>> DataDirManager, etc. that disk  is dead) instead of failing a check.
>>
>> Since this is such a widely used class, this sort of change warrants some
>> discussion, particularly if the functionality would be useful elsewhere.
>>
>> A couple possible implementations:
>>
>> 1. Have a template class that extends Status with a pointer to a
>> heap-allocated T. When handling errors, call a templatized function that
>> downcasts the Status to a StatusWithExtra and returns the T. This is
>> moderately flexible, and wouldn't need to be changed as it gets used more
>> often, but downcasting will warrant careful consideration in terms of
>> type-safety.
>>
>> 2. Have Status have a StatusExtrasPB message that has a number of objects
>> within. As passing data through Status becomes used in other areas of
>> code,
>> the StatusExtrasPB can be extended. This would avoid downcasting,
>> extending
>> Status directly.
>>
>> These are just a couple of ideas, but I'd be interested in seeing what
>> people think and whether anyone had other ideas for this sort of
>> message-passing.
>>
>>
>> Andrew
>>
>>
>

[ANNOUNCE] Jordan Birdsell joining the Kudu PMC

[David Alves]

Congrats Jordan! Well deserved indeed.

-david

> Congrats Jordan! Great work. -Will

Re: Flaky tablet_history_gc-itest

[David Alves]

Your help would be most welcome Mike.
Didn't have to look at it yet, but share Todd's concern that this is
worrying if single node.

-david

On Wed, Oct 12, 2016 at 5:39 PM, Mike Percy  wrote:

> Please let me know if you want me to dig into this; I likely won't have any
> large chunks of time to spend on this before Monday, though.
>
> Mike
>
> --
> Mike Percy
> Software Engineer, Cloudera
>
>
> On Wed, Oct 12, 2016 at 11:23 PM, David Alves 
> wrote:
>
> > No, you're right. I was looking at another test in that file.
> > Hum, then the opportunity for strange things to happen is much lower.
> > Will look more closely.
> >
> > -david
> >
> > On Wed, Oct 12, 2016 at 1:59 PM, Alexey Serbin 
> > wrote:
> >
> > > Hi David,
> > >
> > > Thank you for taking a look at that.
> > > I think the test already uses just one tablet server, so no replicas
> > would
> > > be possible.  I see the following code in the test:
> > >
> > >   StartCluster(1); // Start MiniCluster with a single tablet server.
> > >
> > >   TestWorkload workload(cluster_.get());
> > >
> > >   workload.set_num_replicas(1);
> > >
> > >   workload.Setup(); // Convenient way to create a table.
> > >
> > >
> > >
> > > Did I miss something?  I.e. should I toggle just another control knob
> > > somewhere?
> > >
> > >
> > > Thanks,
> > >
> > > Alexey
> > >
> > > On Wed, Oct 12, 2016 at 1:43 PM, David Alves 
> > > wrote:
> > >
> > > > Hi Alexey
> > > >
> > > > Thanks for going down the rabbit hole.
> > > > Could you try your patch without tablet replication for that test? If
> > the
> > > > problem persists it's unlikely that it's related to the current
> > > consistency
> > > > gaps we have.
> > > > I'm a bit suspicious in that it seems to be doing snapshot scans
> > without
> > > > retrying, which is what we're doing pretty much everywhere else to
> work
> > > > around our gaps.
> > > >
> > > > -david
> > > >
> > > > On Wed, Oct 12, 2016 at 1:06 PM, Alexey Serbin  >
> > > > wrote:
> > > >
> > > > > One small update: the issue might be not in GC logic, but some
> other
> > > > > flakiness related to reading data at snapshot.
> > > > >
> > > > > I updated the patch so the only operations the test now does are
> > > inserts,
> > > > > updates and scans. No tablet merge compactions, redo delta
> > compactions,
> > > > > forced re-updates of missing deltas, or moving time forward.  The
> > > updated
> > > > > patch can be found at:
> > > > >   https://gist.github.com/alexeyserbin/
> > 06ed8dbdb0e8e9abcbde2991c66156
> > > 60
> > > > >
> > > > > The test firmly fails if running as described in the previous
> message
> > > in
> > > > > this thread, just use the updated patch location.
> > > > >
> > > > > David, may be you can take a quick look at that as well?
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Alexey
> > > > >
> > > > > On Wed, Oct 12, 2016 at 2:01 AM, Alexey Serbin <
> aser...@cloudera.com
> > >
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I played with the test (mostly in background), making the failure
> > > > almost
> > > > > > 100% reproducible.
> > > > > >
> > > > > > After collecting some evidence, I can say it's a server-side bug.
> > I
> > > > > think
> > > > > > so because the reproduction scenario I'm talking about uses good
> > old
> > > > > > MANUAL_FLUSH mode, not AUTO_FLUSH_BACKGROUND mode.  Yes, I've
> > > modified
> > > > > the
> > > > > > test slightly to achieve higher reproduction ratio and to clear
> the
> > > > > > question whether it's AUTO_FLUSH_BACKGROUND-specific bug.
> > > > > >
> > > > > > That's what I found:
> > > > > >   1. The problem occurs when updating rows with the same primary
> > 

Re: Flaky tablet_history_gc-itest

[David Alves]

No, you're right. I was looking at another test in that file.
Hum, then the opportunity for strange things to happen is much lower.
Will look more closely.

-david

On Wed, Oct 12, 2016 at 1:59 PM, Alexey Serbin  wrote:

> Hi David,
>
> Thank you for taking a look at that.
> I think the test already uses just one tablet server, so no replicas would
> be possible.  I see the following code in the test:
>
>   StartCluster(1); // Start MiniCluster with a single tablet server.
>
>   TestWorkload workload(cluster_.get());
>
>   workload.set_num_replicas(1);
>
>   workload.Setup(); // Convenient way to create a table.
>
>
>
> Did I miss something?  I.e. should I toggle just another control knob
> somewhere?
>
>
> Thanks,
>
> Alexey
>
> On Wed, Oct 12, 2016 at 1:43 PM, David Alves 
> wrote:
>
> > Hi Alexey
> >
> > Thanks for going down the rabbit hole.
> > Could you try your patch without tablet replication for that test? If the
> > problem persists it's unlikely that it's related to the current
> consistency
> > gaps we have.
> > I'm a bit suspicious in that it seems to be doing snapshot scans without
> > retrying, which is what we're doing pretty much everywhere else to work
> > around our gaps.
> >
> > -david
> >
> > On Wed, Oct 12, 2016 at 1:06 PM, Alexey Serbin 
> > wrote:
> >
> > > One small update: the issue might be not in GC logic, but some other
> > > flakiness related to reading data at snapshot.
> > >
> > > I updated the patch so the only operations the test now does are
> inserts,
> > > updates and scans. No tablet merge compactions, redo delta compactions,
> > > forced re-updates of missing deltas, or moving time forward.  The
> updated
> > > patch can be found at:
> > >   https://gist.github.com/alexeyserbin/06ed8dbdb0e8e9abcbde2991c66156
> 60
> > >
> > > The test firmly fails if running as described in the previous message
> in
> > > this thread, just use the updated patch location.
> > >
> > > David, may be you can take a quick look at that as well?
> > >
> > >
> > > Thanks,
> > >
> > > Alexey
> > >
> > > On Wed, Oct 12, 2016 at 2:01 AM, Alexey Serbin 
> > > wrote:
> > >
> > > > Hi,
> > > >
> > > > I played with the test (mostly in background), making the failure
> > almost
> > > > 100% reproducible.
> > > >
> > > > After collecting some evidence, I can say it's a server-side bug.  I
> > > think
> > > > so because the reproduction scenario I'm talking about uses good old
> > > > MANUAL_FLUSH mode, not AUTO_FLUSH_BACKGROUND mode.  Yes, I've
> modified
> > > the
> > > > test slightly to achieve higher reproduction ratio and to clear the
> > > > question whether it's AUTO_FLUSH_BACKGROUND-specific bug.
> > > >
> > > > That's what I found:
> > > >   1. The problem occurs when updating rows with the same primary keys
> > > > multiple times.
> > > >   2. It's crucial to flush (i.e. call KuduSession::Flush() or
> > > > KuduSession::FlushAsync()) freshly applied update operations not just
> > > once
> > > > in the very end of a client session, but multiple times while adding
> > > those
> > > > operations.  If flushing just once in the very end, the issue becomes
> > 0%
> > > > reproducible.
> > > >   3. The more updates for different rows we have, the more likely we
> > hit
> > > > the issue (but there should be at least a couple updates for every
> > row).
> > > >   4. The problem persists in all types of Kudu builds: debug, TSAN,
> > > > release, ASAN (in the decreasing order of the reproduction ratio).
> > > >   5. The problem is also highly reproducible if running the test via
> > the
> > > > dist_test.py utility (check for 256 out of 256 failure ratio at
> > > > http://dist-test.cloudera.org//job?job_id=aserbin.1476258983.2603 )
> > > >
> > > > To build the modified test and run the reproduction scenario:
> > > >   1. Get the patch from https://gist.github.com/alexeyserbin/
> > > > 7c885148dadff8705912f6cc513108d0
> > > >   2. Apply the patch to the latest Kudu source from the master
> branch.
> > > >   3. Build debug, TSAN, release or ASAN configuration and run with
> the
> > > > command (the rando

Re: Flaky tablet_history_gc-itest

[David Alves]

Hi Alexey

Thanks for going down the rabbit hole.
Could you try your patch without tablet replication for that test? If the
problem persists it's unlikely that it's related to the current consistency
gaps we have.
I'm a bit suspicious in that it seems to be doing snapshot scans without
retrying, which is what we're doing pretty much everywhere else to work
around our gaps.

-david

On Wed, Oct 12, 2016 at 1:06 PM, Alexey Serbin  wrote:

> One small update: the issue might be not in GC logic, but some other
> flakiness related to reading data at snapshot.
>
> I updated the patch so the only operations the test now does are inserts,
> updates and scans. No tablet merge compactions, redo delta compactions,
> forced re-updates of missing deltas, or moving time forward.  The updated
> patch can be found at:
>   https://gist.github.com/alexeyserbin/06ed8dbdb0e8e9abcbde2991c6615660
>
> The test firmly fails if running as described in the previous message in
> this thread, just use the updated patch location.
>
> David, may be you can take a quick look at that as well?
>
>
> Thanks,
>
> Alexey
>
> On Wed, Oct 12, 2016 at 2:01 AM, Alexey Serbin 
> wrote:
>
> > Hi,
> >
> > I played with the test (mostly in background), making the failure almost
> > 100% reproducible.
> >
> > After collecting some evidence, I can say it's a server-side bug.  I
> think
> > so because the reproduction scenario I'm talking about uses good old
> > MANUAL_FLUSH mode, not AUTO_FLUSH_BACKGROUND mode.  Yes, I've modified
> the
> > test slightly to achieve higher reproduction ratio and to clear the
> > question whether it's AUTO_FLUSH_BACKGROUND-specific bug.
> >
> > That's what I found:
> >   1. The problem occurs when updating rows with the same primary keys
> > multiple times.
> >   2. It's crucial to flush (i.e. call KuduSession::Flush() or
> > KuduSession::FlushAsync()) freshly applied update operations not just
> once
> > in the very end of a client session, but multiple times while adding
> those
> > operations.  If flushing just once in the very end, the issue becomes 0%
> > reproducible.
> >   3. The more updates for different rows we have, the more likely we hit
> > the issue (but there should be at least a couple updates for every row).
> >   4. The problem persists in all types of Kudu builds: debug, TSAN,
> > release, ASAN (in the decreasing order of the reproduction ratio).
> >   5. The problem is also highly reproducible if running the test via the
> > dist_test.py utility (check for 256 out of 256 failure ratio at
> > http://dist-test.cloudera.org//job?job_id=aserbin.1476258983.2603 )
> >
> > To build the modified test and run the reproduction scenario:
> >   1. Get the patch from https://gist.github.com/alexeyserbin/
> > 7c885148dadff8705912f6cc513108d0
> >   2. Apply the patch to the latest Kudu source from the master branch.
> >   3. Build debug, TSAN, release or ASAN configuration and run with the
> > command (the random seed is not really crucial, but this gives better
> > results):
> > ../../build-support/run-test.sh ./bin/tablet_history_gc-itest
> > --gtest_filter=RandomizedTabletHistoryGcITest
> .TestRandomHistoryGCWorkload
> > --stress_cpu_threads=64 --test_random_seed=1213726993
> >
> > 4. If running via dist_test.py, run the following instead:
> >
> > ../../build-support/dist_test.py loop -n 256 --
> > ./bin/tablet_history_gc-itest --gtest_filter=
> > RandomizedTabletHistoryGcITest.TestRandomHistoryGCWorkload
> > --stress_cpu_threads=8 --test_random_seed=1213726993
> >
> > Mike, it seems I'll need your help to troubleshoot/debug this issue
> > further.
> >
> >
> > Best regards,
> >
> > Alexey
> >
> >
> > On Mon, Oct 3, 2016 at 9:48 AM, Alexey Serbin 
> > wrote:
> >
> >> Todd,
> >>
> >> I apologize for the late response -- somehow my inbox is messed up.
> >> Probably, I need to switch to use stand-alone mail application (as
> iMail)
> >> instead of browser-based one.
> >>
> >> Yes, I'll take a look at that.
> >>
> >>
> >> Best regards,
> >>
> >> Alexey
> >>
> >> On Mon, Sep 26, 2016 at 12:58 PM, Todd Lipcon 
> wrote:
> >>
> >>> This test has gotten flaky with a concerning failure mode (seeing
> >>> "wrong" results, not just a timeout or something):
> >>>
> >>> http://dist-test.cloudera.org:8080/test_drilldown?test_name=
> >>> tablet_history_gc-itest
> >>>
> >>> It seems like it got flaky starting with Alexey's
> >>> commit bc14b2f9d775c9f27f2e2be36d4b03080977e8fa which switched it to
> >>> use AUTO_FLUSH_BACKGROUND. So perhaps the bug is actually a client bug
> and
> >>> not anything to do with GC.
> >>>
> >>> Alexey, do you have time to take a look, and perhaps consult with Mike
> >>> if you think it's actually a server-side bug?
> >>>
> >>> -Todd
> >>>
> >>> --
> >>> Todd Lipcon
> >>> Software Engineer, Cloudera
> >>>
> >>
> >>
> >
>

Re: [VOTE] Apache Kudu 1.0.1 RC1

[David Alves]

+0 Compiled thirdparty successfully, but couldn't compile the sources in
macOS Sierra/XCode 8 as this is missing
https://gerrit.cloudera.org/#/c/4482/ and
https://gerrit.cloudera.org/#/c/4495/

Since this is just a bug fix/doc update release and this problem only
affects macOS (macOS is only supported for development and development is
usually made on trunk) I don't think this issue should hold the release.

-david


On Mon, Oct 10, 2016 at 1:54 PM, Dan Burkert  wrote:

> +1
>
> * Compiled and tested release mode on OS X 10.10, with no unexpected
> failures.
> * Compiled and tested Debug/ASAN/TSAN/Release on Centos 6, no failures.
> * Checked signatures and licenses (RAT)
>
> - Dan
>
> On Mon, Oct 10, 2016 at 12:27 PM, Alexey Serbin 
> wrote:
>
> > +1
> >
> > I compiled the project from source using the 1.0.1-RC1 tag (release and
> > debug modes) and ran the test suite using 'ctest -j4' at CentOS 6.6
> (kernel
> > 2.6.32-504.30.3.el6.x86_64).  All tests passed.
> >
> > Also, I compiled the project from source using the 1.0.1-RC1 tag (release
> > and debug modes) and ran the tests using 'ctest -j2' at MacOS X 10.11.5.
> > All tests passed except for pstack_watcher-test, which is a known issue
> for
> > Kudu on MacOS X.
> >
> > Besides, I run 'kudu test loadgen' tool against the newly built binaries
> > multiple times, inserting several million of rows and it passed.
> >
> >
> > Best regards,
> >
> > Alexey
> >
> >
> > On Fri, Oct 7, 2016 at 2:00 PM, Dan Burkert 
> wrote:
> >
> > > Hi,
> > >
> > > We're happy to announce the first release candidate for Apache Kudu
> > 1.0.1.
> > >
> > > This release includes bug fixes and documentation updates since the
> 1.0.0
> > > release.
> > >
> > > The is a source-only release. The artifacts were staged here:
> > > https://dist.apache.org/repos/dist/dev/kudu/1.0.1-RC1/
> > >
> > > It was built from this tag:
> > > https://git-wip-us.apache.org/repos/asf?p=kudu.git;a=commit;h=
> > > e60b610253f4303b24d41575f7bafbc5d69edddb
> > >
> > > The release notes can be found here (the release notes on
> > kudu.apache.org
> > > will be updated with the release):
> > > https://github.com/apache/kudu/blob/branch-1.0.x/docs/
> release_notes.adoc
> > >
> > > KEYS file:
> > > https://www.apache.org/dist/kudu/KEYS
> > >
> > > I'd suggest going through the README, building Kudu, and running the
> > > unit tests.
> > >
> > > Please try the release and vote; vote will be open for at least 72
> hours.
> > >
> > > Thanks,
> > >
> > > - Dan
> > >
> >
>

Re: Kudu 1.0.1 patch release

[David Alves]

I'd like to get "Address alter_table_randomized-test flakyness on
ResultTracker" https://gerrit.cloudera.org/#/c/4629/ merged in. Rare race
but potential for SIGSEGV on a highly battered cluster.
Patch/fix/bug is simple enough that side-effects are very unlikely if not
impossible.

-david

On Thu, Oct 6, 2016 at 12:12 PM, Dan Burkert  wrote:

> I'll make sure we release a new build of the website since a lot of docs
> have changed.  My understanding is that this can be decoupled from the
> actual release branch, so doc-only changes don't need to be backported.
>
> There has been a big effort over the last week and a half to pay down some
> of the flaky tests.  This has resulted in some new bug fix patches which
> are not (as of now) included in the 1.0.1 branch:
>
> * mem_tracker: fix race between FindTracker() and destructor
>  02888ba994>
> * [java client] Tight-ish loop in master lookups if a tablet doesn't ha…
>  6aafdc2bdd>
> * KUDU-1681: DNS resolution failure of master hostname causes tserver crash
> 
> * Address alter_table_randomized-test flakyness on ResultTracker
> 
> * log: address a heap overflow race during log roll
> 
>
> Of these, I believe only KUDU-1681 has been observed outside of tests.  Any
> thoughts/opinions on adding some or all of these to the 1.0.1 release?
>
> - Dan
>
> On Wed, Oct 5, 2016 at 9:17 AM, Mike Percy  wrote:
>
> > Dan thanks for RMing!
> >
> > I think it would be useful to also pull in docs patches that seem to have
> > missed 1.0.0 like d6c5507049757735cc88659f919a5a5d12092da6
> > and fcbfa90e4a594c473901297a47dfa2d01f1229e1
> >
> > Mike
> >
> > On Tue, Oct 4, 2016 at 1:12 AM, Dan Burkert 
> wrote:
> >
> > > Reminder that we are going to cut the 1.0.1 RC this Friday.  If you
> have
> > > any additional patches you want to include now is the time to speak up.
> > > Besides the issues already discussed, the following should make the
> cut:
> > >
> > > KUDU-1660 / 69e657
> > >  > > 7b59595c0e>:
> > > Kudu fails to start up on single CPU system
> > > KUDU-1652 part 2 
> > >
> > > - Dan
> > >
> > > On Tue, Sep 27, 2016 at 11:22 AM, Dan Burkert 
> > > wrote:
> > >
> > > > Yep, I can take care of that.
> > > >
> > > > - Dan
> > > >
> > > > On Tue, Sep 27, 2016 at 11:21 AM, Todd Lipcon 
> > wrote:
> > > >
> > > >> Sounds good. Are you going to take care of cherry-picking these
> items
> > to
> > > >> branch-1.0.x? We also need a commit to reset the version in that
> > branch
> > > to
> > > >> 1.0.1-SNAPSHOT during the dev period.
> > > >>
> > > >> On Tue, Sep 27, 2016 at 11:11 AM, Dan Burkert <
> danburk...@apache.org>
> > > >> wrote:
> > > >>
> > > >> > Fixes have landed for KUDU-1651
> > > >> >  > 1f40913de6d3427847f3d435e1b84f
> > > >> > b928a6f6a9>
> > > >> > and KUDU-1652
> > > >> >  > ce17a9c4eb34dcfe63c8d4321d38d1
> > > >> > 8a0cb8c5c2>,
> > > >> > so every outstanding known issue
> > > >> > that we want to address has a committed fix in master. Since many
> > > people
> > > >> > are
> > > >> > otherwise busy with the Strata conference this week, I think we
> > should
> > > >> > stick to
> > > >> > the original plan to cut an RC on October 7th, with a potential
> > > release
> > > >> > date the
> > > >> > following week. If any further bugs arise before the 7th we can
> > > consider
> > > >> > them
> > > >> > for inclusion.  Thanks all.
> > > >> >
> > > >> > - Dan
> > > >> >
> > > >> > On Mon, Sep 26, 2016 at 11:18 AM, Adar Dembo 
> > > wrote:
> > > >> > > +1 to a patch release.
> > > >> > >
> > > >> > > My only opinion on the nominated patches is that I'm not sure if
> > we
> > > >> > > need to include the KUDU-1090 fix. IIRC it was very rare and
> > > generally
> > > >> > > only contributed to test flakiness. But, I'm fine with including
> > it
> > > if
> > > >> > > others feel strongly.
> > > >> > >
> > > >> > > On Mon, Sep 26, 2016 at 10:55 AM, Todd Lipcon <
> t...@cloudera.com>
> > > >> wrote:
> > > >> > >> +1 for a bug fix release, especially to address the known crash
> > > bugs.
> > > >> > I'll
> > > >> > >> add one more to the running:
> > > >> > >>
> > > >> > >> https://gerrit.cloudera.org/#/c/4535/
> > > >> > >>
> > > >> > >> This fixes a crash when running on single-core systems. Again
> not
> > > too
> > > >> > >> common (even most VMs have two cores these days) but it can be
> a
> > > >> blocker
> > > >> > if
> > > >> > >> you need to run on a single-core.
> > > >> > >>
> > > >> > >> -Todd
> > > >> > >>
> > > >> > >> On Mon, Sep 26, 2016 at 10:49 AM, Dan Burkert <
> > > danburk...@apache.org
> > > >> >
> > > >> 

Re: Initial draft of consistency/isolation scoping document

[David Alves]

Same doc, shortened/unbroken URL:
https://s.apache.org/7VCo

-david

On Tue, Oct 4, 2016 at 5:11 PM, David Alves  wrote:

> Hi All
>
>   Some users have manifested interest/concern about consistency/isolation
> in Kudu. I've written a doc that describes what we're missing and tries to
> suggest some kind of roadmap:
>
>   https://docs.google.com/document/d/1EaKlJyQdMBz6G-Xn5uktY-
> d_x0uRmjMCrDGP5rZ7AoI/edit?usp=sharing
>
>   It's a work in progress, but please take a read, if you're interested,
> and feel free to make suggestions, ask questions, etc.
>
> Best
> David
>

Initial draft of consistency/isolation scoping document

[David Alves]

Hi All

  Some users have manifested interest/concern about consistency/isolation
in Kudu. I've written a doc that describes what we're missing and tries to
suggest some kind of roadmap:

  https://docs.google.com/document/d/1EaKlJyQdMBz6G-
Xn5uktY-d_x0uRmjMCrDGP5rZ7AoI/edit?usp=sharing

  It's a work in progress, but please take a read, if you're interested,
and feel free to make suggestions, ask questions, etc.

Best
David

Re: Translating Kudu documentation from English into Brazilian Portuguese

[David Alves]

Hi Adalberto

  I share Todd's concern about maintenance, particularly if we're setting
out to translate the whole set.
  As native portuguese speaker though, I'd love to see more portuguese
writing on Kudu. I'd be happy to review blog posts or any other kind of
written format contribution, just let me know.

Best
David

On Thu, Sep 22, 2016 at 12:29 PM, Todd Lipcon  wrote:

> Hi Adalberto,
>
> We don't have any foreign language translations of the documentation at
> this time. One fear I have is that the translated documentation will
> quickly become out-of-date. For example, if you translate the docs now, do
> you think you'll also be around to translate the docs for future releases?
>
> My fear is based on what happened with a similar effort in the Apache
> Hadoop project. At one point in late 2008, some contributors stepped up to
> translate the documentation into Chinese. Then by mid 2011, the
> documentation had seen no maintenance and was causing confusion -- Chinese
> users were reading the documentation for old versions which contained
> information that was not accurate for the versions that had been released
> in recent years. We ended up removing the translated documentation at that
> point.
>
> I won't veto the contribution, but would be interested to hear what other
> developers in the community think about this.
>
> If the goal is to increase adoption of Kudu in Brazil, maybe another way to
> help would be to write a Brazilian Portuguese tutorial/introduction blog
> post? Or give some talks at local meetups and user groups? I presented once
> at SouJava and found the audience was really great. Maybe we can get in
> touch with the organizers to propose a session?
>
> Thanks
> -Todd
>
>
>
> On Thu, Sep 22, 2016 at 10:01 AM,  wrote:
>
> > Dear All,
> >
> >
> >
> > I’d like to assist on translating the Kudu documentation into Brazilian
> > Portuguese. What is necessary to contribute on this?
> >
> >
> >
> > Regards,
> >
> >
> >
> > Adalberto Sávio Rodrigues
> >
> > São Paulo - Brazil
> >
> > E-mail: adalberto.s.rodrig...@gmail.com
> >
> > Mobile / WhatsApp: +55 11 975779955
> >
> > Skype: adalsr
> >
> >   https://br.linkedin.com/in/adalsr
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > ---
> > Este email foi escaneado pelo Avast antivírus.
> > https://www.avast.com/antivirus
> >
>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>

RFC: removing raft_consensus-test (mock-based)

[David Alves]

+1, we've since broadened our coverage in other tests that those cases
are covered elsewhere. Plus I agree that with the queue taking more
responsibility mocking it becomes a hardship.

-david


> I'm doing a bit of Consensus refactoring, and finding that the 
> raft_consensus-test mock-based test is a bit of a roadblock to refactoring. 
> We don't use gmock anywhere else, and so whenever I look at this code I 
> quickly develop a head-ache trying to understand what it's doing. Looking 
> through the test cases themselves, I believe all of the code paths they try 
> to cover are equally well covered by other test cases in 
> raft_consensus-itest. Rather than spend several hours trying to figure out 
> why the mock isn't behaving like the real code, I'd rather spend that time on 
> adding more test cases for the real code. Thoughts?

Re: 'tidy-bot' now running (experimentally)

[David Alves]

+1 to keep, abbreviating from the definition seems evil.

-david

On Mon, Sep 19, 2016 at 2:19 PM, Adar Dembo  wrote:

> Personally I haven't seen much of a need for abbreviated parameter
> names. I'd vote for keeping the warning.
>
> On Mon, Sep 19, 2016 at 2:14 PM, Todd Lipcon  wrote:
> > How do people feel about this warning?
> >
> > warning: function 'kudu::InsertLoadgen::InserterThread' has a definition
> > with different parameter names
> > [readability-inconsistent-declaration-parameter-name]
> > void InserterThread(Generator::Mode gen_mode, int64_t seed,
> > ^
> > gen_seed
> > src/kudu/tools/insert-generated-rows.cc:307:21: note: the definition
> seen
> > here
> > void InsertLoadgen::InserterThread(Generator::Mode gen_mode, int64_t
> > gen_seed,
> > ^
> > src/kudu/tools/insert-generated-rows.cc:234:8: note: differing
> parameters
> > are named here: ('seed'), in definition: ('gen_seed')
> > void InserterThread(Generator::Mode gen_mode, int64_t seed,
> >
> >
> > Is it a reasonable one to keep, or should I disable it? On the one hand,
> it
> > might catch a bug where the order of parameters is swapped between the
> > definition and declaration, or where you forgot to update the name when
> > changing the definition. On the other hand, sometimes it can make sense
> to
> > have an "abbreviated" parameter name in the function definition.
> >
> > -Todd
> >
> > On Fri, Sep 16, 2016 at 10:42 AM, Adar Dembo  wrote:
> >
> >> OK, let's keep it then. I changed the comment in question to be
> >> "TODO(KUDU-1537)...".
> >>
> >> On Thu, Sep 15, 2016 at 8:18 PM, Jake Farrell 
> wrote:
> >> > we use "TODO(bug id or committer id): msg" as the format in other
> >> projects
> >> > and that seems to be enough breadcrumb in most cases
> >> >
> >> > -Jake
> >> >
> >> > On Thu, Sep 15, 2016 at 11:06 PM, Todd Lipcon 
> wrote:
> >> >
> >> >> On Thu, Sep 15, 2016 at 6:50 PM, Adar Dembo 
> wrote:
> >> >>
> >> >> > In https://gerrit.cloudera.org/#/c/4435/, Tidy Bot said:
> >> >> >
> >> >> > Line 209:   // TODO: Should be fixed with Exactly Once
> semantics,
> >> >> > see KUDU-1537.
> >> >> > warning: missing username/bug in TODO [google-readability-todo]
> >> >> >   // TODO: Should be fixed with Exactly Once semantics, see
> >> >> KUDU-1537.
> >> >> >   ^
> >> >> >   // TODO(unknown): Should be fixed with Exactly Once
> semantics,
> >> >> > see KUDU-1537.
> >> >> >
> >> >> > This doesn't look like the kind of style change we want, right?
> >> >> > Historically we don't annotate our TODOs with names.
> >> >> >
> >> >> > Or should I reformat it as "TODO(KUDU-1537)..." ?
> >> >> >
> >> >>
> >> >> Yea, I think TODO(bug#) is a good policy to try to have moving
> forward,
> >> but
> >> >> I don't think we have to be religious about it. I can turn off this
> tidy
> >> >> check if we think it's not worth it with a codebase of our size.
> >> >>
> >> >> This guideline comes from Google (
> >> >> https://google.github.io/styleguide/cppguide.html#TODO_Comments)
> where
> >> >> they
> >> >> say:
> >> >>
> >> >> TODOs should include the string TODO in all caps, followed by the
> name,
> >> >> e-mail address, bug ID, or other identifier of the person or issue
> with
> >> the
> >> >> best context about the problem referenced by the TODO. The main
> purpose
> >> is
> >> >> to have a consistent TODO that can be searched to find out how to get
> >> more
> >> >> details upon request. A TODO is not a commitment that the person
> >> referenced
> >> >> will fix the problem. Thus when you create a TODO with a name, it is
> >> almost
> >> >> always your name that is given.
> >> >> It's sort of nice to leave a breadcrumb, but it's also not too hard
> for
> >> >> someone to 'git blame' and figure out who added it, so I could go
> either
> >> >> way.
> >> >>
> >> >> -Todd
> >> >>
> >> >>
> >> >> >
> >> >> >
> >> >> > On Wed, Sep 14, 2016 at 10:04 PM, Todd Lipcon 
> >> wrote:
> >> >> > > Hey folks,
> >> >> > >
> >> >> > > I've set up a jenkins job and gerrit trigger to run
> clang-tidy-diff
> >> on
> >> >> > any
> >> >> > > patches that are uploaded. It should be set up now so as not to
> >> vote +1
> >> >> > or
> >> >> > > -1, but just to produce comments. For an example of the type of
> >> >> warnings
> >> >> > it
> >> >> > > generates, check out:
> >> >> > > https://gerrit.cloudera.org/#/c/4409/4/src/kudu/consensus/
> >> >> > raft_consensus_state.h
> >> >> > >
> >> >> > > If you see any checks that you think are false positives, feel
> free
> >> to
> >> >> > ping
> >> >> > > me and I can either disable those checks entirely, or see if
> there's
> >> >> some
> >> >> > > configuration we can make to better match our own guidelines.
> >> >> > >
> >> >> > > Hopefully this turns out to be a useful bit of "automatic code
> >> review"
> >> >> so
> >> >> > > that we can focus our review efforts less on mechanical checks
> and
> >> more
> >> >> > on
> >> >> > > things requiring human judgment :) If it turns out to be more of
> an
> >> >> > > ann

Re: A couple more python patches

[David Alves]

Hi Jordan

  Feel free to add reviewers to your patches (you can do so in gerrit's web
ui, or at push time), that way folks get notified when you do a push (if
they aren't subscribed to the whole set of notifications, for instance).
  Usually we name a couple of folks that we feel are the ones that
understand the component we're working on the best, or if we can't think of
anyone specific, then at random :)
  For now you can add me as a reviewer to your python patches, I've likely
been the last one to work more extensively on it.

Best
David

On Wed, Sep 14, 2016 at 5:17 PM, Jordan Birdsell 
wrote:

> Hey folks,
>
> I've put together a couple more python patches:
>
> For KUDU-1612, KUDU-1615 and KUDU-1615:
> https://gerrit.cloudera.org/#/c/4417/
> This addresses enabling getting/setting timestamps (unixtime_micros),
> scanner read at snapshot and a minor bug with write operations. I combined
> them because of the overlap in functionality, however if this is bad form,
> let me know.
>
> For KUDU-1611:
> https://gerrit.cloudera.org/#/c/4408/
> This enables setting the selection mode for scanners/scan token builders.
>
> Someone mind taking a look?
>
> Thanks,
> Jordan
>

Re: [VOTE] Apache Kudu 1.0.0 RC1

[David Alves]

+1

Downloaded and built on mac osx el capitan.
Ran all tests (in serial), all passed.

-david

On Tue, Sep 13, 2016 at 12:39 AM, Todd Lipcon  wrote:

> Hi,
>
> The Apache Kudu team is happy to announce the first release candidate for
> Apache Kudu 1.0.0.
>
> After approximately a year of beta releases, Apache Kudu has reached
> version 1.0.
> This version number signifies that the development team feels that Kudu is
> stable
> enough for usage in production environments.
>
> The is a source-only release. The artifacts were staged here:
> https://dist.apache.org/repos/dist/dev/kudu/1.0.0-RC1/
>
> Java convenience binaries in the form of a Maven repository are staged
> here:
> https://repository.apache.org/content/repositories/orgapachekudu-1001/
>
> It was built from this tag:
> https://git-wip-us.apache.org/repos/asf?p=kudu.git;a=commit;h=
> 6f6e49ca98c3e3be7d81f88ab8a0f9173959b191
>
> The release notes can be found here (some links from this document will
> only work when this version is released):
> https://github.com/apache/kudu/blob/branch-1.0.x/docs/release_notes.adoc
>
> KEYS file:
> http://www.apache.org/dist/kudu/KEYS
>
> I'd suggest going through the README, building Kudu, and running the
> unit tests. Testing out the Maven repo would also be appreciated.
>
> Please try the release and vote; the vote will end in 72 hours (on
> 9/16/2016 at 12:30am PDT).
>
> Thanks,
> Todd
>

Re: [ANNOUNCE] Two new Kudu committer/PMC members

[David Alves]

Congrats Alexey and Will!!

On Mon, Sep 12, 2016 at 3:55 PM, Todd Lipcon  wrote:

> Hi Kudu community,
>
> It's my great pleasure to announce that the PMC has voted to add both
> Alexey Serbin and Will Berkeley as committers and PMC members.
>
> Alexey has been contributing for a few months, including developing some
> pretty meaty (and tricky) additions. Two of note are the addition of
> doxygen for our client APIs, as well as the implementation of
> AUTO_FLUSH_BACKGROUND in C++. He has also been quite active in reviews
> recently, having reviewed 40+ patches in the last couple months. He also
> contributed by testing and voting on the recent 0.10 release.
>
> Will has been a great contributor as well, spending a lot of time in areas
> that don't get as much love from others. In particular, he's made several
> fixes to the web UIs, has greatly improved the Flume integration, and has
> been burning down a lot of long-standing bugs recently. Will also spends a
> lot of his time outside of Kudu working with users and always has good
> input on what our user community will think of a feature. Like Alexey, Will
> also participated in the 0.10 release process.
>
> Both of these community members have already been "acting the part"
> through their contributions detailed above, and the PMC is excited to
> continue working with them in their expanded roles.
>
> Please join me in congratulating them!
>
> -Todd
>

Re: Potential Java client issue?

[David Alves]

Look at it for a bit and think I figured out the problem and fixed it (put
on the ip2client map is synchronized with the put on client2tablets so it
makes sense that the removing is too).
The problem is that I'm having a hard time reproing the first failure.
I'll ask JD's opinion tomorrow.

-david

On Sun, Sep 11, 2016 at 8:25 PM, Todd Lipcon  wrote:

> I just had a Jenkins job time out when TestAsyncKuduSession java test took
> an hour:
> http://104.196.14.100/job/kudu-gerrit/3359/BUILD_TYPE=RELEASE/console
>
> Looking at the test output
> http://104.196.14.100/job/kudu-gerrit/3359/BUILD_TYPE=
> RELEASE/artifact/java/kudu-client/target/surefire-reports/org.apache.kudu.
> client.TestAsyncKuduSession-output.txt
> ,
> I see the following NPE after an unexpected disconnect:
>
> 00:50:11.056 [WARN - main] (AsyncKuduSession.java:334) unexpected
> tablet lookup failure for operation KuduRpc(method=Write, tablet=null,
> attempt=0, DeadlineTracker(timeout=0, elapsed=32),
> Deferred@1645276019(state=PENDING, result=null, callback=,
> errback=)) row_key=(int32 key=1)
> java.lang.NullPointerException
> at org.apache.kudu.client.AsyncKuduClient$RemoteTablet.
> addTabletClient(AsyncKuduClient.java:2089)
> at org.apache.kudu.client.AsyncKuduClient$RemoteTablet.
> refreshTabletClients(AsyncKuduClient.java:2049)
> at org.apache.kudu.client.AsyncKuduClient.discoverTablets(
> AsyncKuduClient.java:1404)
> at org.apache.kudu.client.AsyncKuduClient$MasterLookupCB.call(
> AsyncKuduClient.java:1317)
> at org.apache.kudu.client.AsyncKuduClient$MasterLookupCB.call(
> AsyncKuduClient.java:1297)
> at com.stumbleupon.async.Deferred.doCall(Deferred.java:1280)
> at com.stumbleupon.async.Deferred.addCallbacks(Deferred.java:685)
> at com.stumbleupon.async.Deferred.addCallback(Deferred.java:721)
> at org.apache.kudu.client.AsyncKuduClient.locateTablet(
> AsyncKuduClient.java:1106)
> at org.apache.kudu.client.AsyncKuduClient.loopLocateTable(
> AsyncKuduClient.java:1206)
> at org.apache.kudu.client.AsyncKuduClient.locateTable(
> AsyncKuduClient.java:1241)
> at org.apache.kudu.client.AsyncKuduClient.getTabletLocation(
> AsyncKuduClient.java:1450)
> at org.apache.kudu.client.AsyncKuduSession.apply(
> AsyncKuduSession.java:509)
> at org.apache.kudu.client.TestAsyncKuduSession.
> testInsertIntoUnavailableTablet(TestAsyncKuduSession.java:162)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at sun.reflect.NativeMethodAccessorImpl.invoke(
> NativeMethodAccessorImpl.java:57)
> at sun.reflect.DelegatingMethodAccessorImpl.invoke(
> DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(
> FrameworkMethod.java:47)
> at org.junit.internal.runners.model.ReflectiveCallable.run(
> ReflectiveCallable.java:12)
> at org.junit.runners.model.FrameworkMethod.invokeExplosively(
> FrameworkMethod.java:44)
> at org.junit.internal.runners.statements.InvokeMethod.
> evaluate(InvokeMethod.java:17)
> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
>
>
> I'm guessing this is an artifact
> of d5082d8ec1218e3f3bd2143d117ddd64772a6162 which was committed Friday.
> David, since you committed that change, do you mind looking into this and
> decide if we should revert this patch for 1.0 so we have some more time to
> thoroughly test it?
>
> -Todd
> --
> Todd Lipcon
> Software Engineer, Cloudera
>

Re: Projection schema drops key

[David Alves]

Oh I see what you mean.
It's not that the keys are getting dropped, its that they're not marked as
keys.
This arguably makes sense on a projection: for instance you might want the
keys returns in the end of the projection, while table schemas (at least
for now) require that they are present at the beginning of the projection.
If you really want to create a new table based on an existing one, you
could get the schema from KuduTable. That one should be complete.

-david

On Fri, Sep 9, 2016 at 8:51 AM, Jordan Birdsell 
wrote:

> Right, what i'm saying is, if i do include the key in my projection, the
> schema does not maintain it as a key.  The issue isnt so much that i cant
> apply predicates to the key column, its that if i wanted to create a
> projection and then want to use that projection to create a table based on
> that projection, i'd have to rebuild the schema (i.e., the schema returned
> is effectively useless for creating new tables).  This pattern of creating
> tables from projections is pretty common in dataframe like libraries in
> python.
>
> gist of offending code with comment on issue:
> https://gist.github.com/jtbirdsell/e376a7fa21f3b1893efa7e1ddac408d7
>
>
> On Fri, Sep 9, 2016 at 11:38 AM David Alves  wrote:
>
> > Wait, If you _do_ set a projection on the scanner that does not include
> the
> > keys, then they won't be returned (and won't appear on the projection's
> > schema).
> > Note that this does not mean that you can't set predicates on the key,
> it's
> > just that they'll be evaluated server side, but the key won't actually be
> > returned.
> > Maybe I'm misunderstanding what you're saying?
> > Care to post a gist with the offending code?
> >
> > -david
> >
> >
> >
> > On Fri, Sep 9, 2016 at 8:26 AM, Jordan Birdsell <
> jordantbirds...@gmail.com
> > >
> > wrote:
> >
> > > Hey David,
> > >
> > > Yep, i'm sure, taking a look at the scan_configuration class, the issue
> > > seems to be here:
> > >
> > > Status ScanConfiguration::SetProjectedColumnIndexes(const vector&
> > > col_indexes) {
> > > 
> > >   RETURN_NOT_OK*(s->Reset(cols, 0));*
> > > 
> > >
> > > In the SetProjectedColumnIndexes method (which is also used by
> > > SetProjectedColumnNames), we're setting the schema without the index.
> > >
> > > There are probably a couple of ways to address this:
> > >
> > >1. Check if all key columns are in the projection, and if so,
> maintain
> > >the key.
> > >2. Provide an optional parameter to be able to set the key to users
> > >preference for the new projection. This would be beneficial for
> cases
> > > where
> > >the user may want to create a new table based on their projection.
> > >
> > > Thoughts?
> > >
> > > Jordan
> > >
> > > On Fri, Sep 9, 2016 at 11:08 AM David Alves 
> > wrote:
> > >
> > > > Hi Jordan
> > > >
> > > >   KuduScanner::GetProjectionSchema returns the schema of the
> projection
> > > > that was previously set on the scanner. If you don't a projection it
> > > should
> > > > indeed return all the columns.
> > > >   Are you sure you didn't set a projection (with
> > SetProjectedColumnNames
> > > > or SetProjectedColumnIndexes) that excluded the key?
> > > >
> > > > Best
> > > > David
> > > >
> > > > On Fri, Sep 9, 2016 at 5:16 AM, Jordan Birdsell <
> > > jordantbirds...@gmail.com
> > > > >
> > > > wrote:
> > > >
> > > > > Hey folks,
> > > > >
> > > > > i was doing some work on KUDU-854
> > > > > <https://issues.apache.org/jira/browse/KUDU-854> and when testing
> > the
> > > > > KuduScanner::GetProjectionSchema method call, found that the key
> was
> > > > being
> > > > > dropped, which makes this much more challenging to test. Any ideas
> if
> > > it
> > > > is
> > > > > intended to drop the key information in a scanner projection? I
> would
> > > > > imagine this could prevent functionality like creating new tables
> > from
> > > a
> > > > > projection.
> > > > >
> > > > > Thanks,
> > > > > Jordan
> > > > >
> > > >
> > >
> >
>

Re: Projection schema drops key

[David Alves]

I meant a gist with a example of client API usage where the keys are
getting dropped, so that we can reproduce what you're seeing.

-david

On Fri, Sep 9, 2016 at 8:51 AM, Jordan Birdsell 
wrote:

> Right, what i'm saying is, if i do include the key in my projection, the
> schema does not maintain it as a key.  The issue isnt so much that i cant
> apply predicates to the key column, its that if i wanted to create a
> projection and then want to use that projection to create a table based on
> that projection, i'd have to rebuild the schema (i.e., the schema returned
> is effectively useless for creating new tables).  This pattern of creating
> tables from projections is pretty common in dataframe like libraries in
> python.
>
> gist of offending code with comment on issue:
> https://gist.github.com/jtbirdsell/e376a7fa21f3b1893efa7e1ddac408d7
>
>
> On Fri, Sep 9, 2016 at 11:38 AM David Alves  wrote:
>
> > Wait, If you _do_ set a projection on the scanner that does not include
> the
> > keys, then they won't be returned (and won't appear on the projection's
> > schema).
> > Note that this does not mean that you can't set predicates on the key,
> it's
> > just that they'll be evaluated server side, but the key won't actually be
> > returned.
> > Maybe I'm misunderstanding what you're saying?
> > Care to post a gist with the offending code?
> >
> > -david
> >
> >
> >
> > On Fri, Sep 9, 2016 at 8:26 AM, Jordan Birdsell <
> jordantbirds...@gmail.com
> > >
> > wrote:
> >
> > > Hey David,
> > >
> > > Yep, i'm sure, taking a look at the scan_configuration class, the issue
> > > seems to be here:
> > >
> > > Status ScanConfiguration::SetProjectedColumnIndexes(const vector&
> > > col_indexes) {
> > > 
> > >   RETURN_NOT_OK*(s->Reset(cols, 0));*
> > > 
> > >
> > > In the SetProjectedColumnIndexes method (which is also used by
> > > SetProjectedColumnNames), we're setting the schema without the index.
> > >
> > > There are probably a couple of ways to address this:
> > >
> > >1. Check if all key columns are in the projection, and if so,
> maintain
> > >the key.
> > >2. Provide an optional parameter to be able to set the key to users
> > >preference for the new projection. This would be beneficial for
> cases
> > > where
> > >the user may want to create a new table based on their projection.
> > >
> > > Thoughts?
> > >
> > > Jordan
> > >
> > > On Fri, Sep 9, 2016 at 11:08 AM David Alves 
> > wrote:
> > >
> > > > Hi Jordan
> > > >
> > > >   KuduScanner::GetProjectionSchema returns the schema of the
> projection
> > > > that was previously set on the scanner. If you don't a projection it
> > > should
> > > > indeed return all the columns.
> > > >   Are you sure you didn't set a projection (with
> > SetProjectedColumnNames
> > > > or SetProjectedColumnIndexes) that excluded the key?
> > > >
> > > > Best
> > > > David
> > > >
> > > > On Fri, Sep 9, 2016 at 5:16 AM, Jordan Birdsell <
> > > jordantbirds...@gmail.com
> > > > >
> > > > wrote:
> > > >
> > > > > Hey folks,
> > > > >
> > > > > i was doing some work on KUDU-854
> > > > > <https://issues.apache.org/jira/browse/KUDU-854> and when testing
> > the
> > > > > KuduScanner::GetProjectionSchema method call, found that the key
> was
> > > > being
> > > > > dropped, which makes this much more challenging to test. Any ideas
> if
> > > it
> > > > is
> > > > > intended to drop the key information in a scanner projection? I
> would
> > > > > imagine this could prevent functionality like creating new tables
> > from
> > > a
> > > > > projection.
> > > > >
> > > > > Thanks,
> > > > > Jordan
> > > > >
> > > >
> > >
> >
>

Re: Projection schema drops key

[David Alves]

Wait, If you _do_ set a projection on the scanner that does not include the
keys, then they won't be returned (and won't appear on the projection's
schema).
Note that this does not mean that you can't set predicates on the key, it's
just that they'll be evaluated server side, but the key won't actually be
returned.
Maybe I'm misunderstanding what you're saying?
Care to post a gist with the offending code?

-david



On Fri, Sep 9, 2016 at 8:26 AM, Jordan Birdsell 
wrote:

> Hey David,
>
> Yep, i'm sure, taking a look at the scan_configuration class, the issue
> seems to be here:
>
> Status ScanConfiguration::SetProjectedColumnIndexes(const vector&
> col_indexes) {
> 
>   RETURN_NOT_OK*(s->Reset(cols, 0));*
> 
>
> In the SetProjectedColumnIndexes method (which is also used by
> SetProjectedColumnNames), we're setting the schema without the index.
>
> There are probably a couple of ways to address this:
>
>1. Check if all key columns are in the projection, and if so, maintain
>the key.
>2. Provide an optional parameter to be able to set the key to users
>preference for the new projection. This would be beneficial for cases
> where
>the user may want to create a new table based on their projection.
>
> Thoughts?
>
> Jordan
>
> On Fri, Sep 9, 2016 at 11:08 AM David Alves  wrote:
>
> > Hi Jordan
> >
> >   KuduScanner::GetProjectionSchema returns the schema of the projection
> > that was previously set on the scanner. If you don't a projection it
> should
> > indeed return all the columns.
> >   Are you sure you didn't set a projection (with SetProjectedColumnNames
> > or SetProjectedColumnIndexes) that excluded the key?
> >
> > Best
> > David
> >
> > On Fri, Sep 9, 2016 at 5:16 AM, Jordan Birdsell <
> jordantbirds...@gmail.com
> > >
> > wrote:
> >
> > > Hey folks,
> > >
> > > i was doing some work on KUDU-854
> > > <https://issues.apache.org/jira/browse/KUDU-854> and when testing the
> > > KuduScanner::GetProjectionSchema method call, found that the key was
> > being
> > > dropped, which makes this much more challenging to test. Any ideas if
> it
> > is
> > > intended to drop the key information in a scanner projection? I would
> > > imagine this could prevent functionality like creating new tables from
> a
> > > projection.
> > >
> > > Thanks,
> > > Jordan
> > >
> >
>

Re: Projection schema drops key

[David Alves]

Hi Jordan

  KuduScanner::GetProjectionSchema returns the schema of the projection
that was previously set on the scanner. If you don't a projection it should
indeed return all the columns.
  Are you sure you didn't set a projection (with SetProjectedColumnNames
or SetProjectedColumnIndexes) that excluded the key?

Best
David

On Fri, Sep 9, 2016 at 5:16 AM, Jordan Birdsell 
wrote:

> Hey folks,
>
> i was doing some work on KUDU-854
>  and when testing the
> KuduScanner::GetProjectionSchema method call, found that the key was being
> dropped, which makes this much more challenging to test. Any ideas if it is
> intended to drop the key information in a scanner projection? I would
> imagine this could prevent functionality like creating new tables from a
> projection.
>
> Thanks,
> Jordan
>

Re: KUDU-1593 Patch

[David Alves]

Hi Jordan

  Thanks for contributing.
  I've left some comments.

Best
David

On 9/6/16, Jordan Birdsell  wrote:
> Hey all,
>
> I've submitted a patch for KUDU-1593
> . This patch exposes the
> num_replicas method for the KuduTableCreator class to the python api.
> Currently, all tables created in python are created with the default number
> of replicas.
>
> Can someone please review?
>
> https://gerrit.cloudera.org/#/c/4315/
>
> Thanks,
> Jordan Birdsell
>