impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Manaswini Maharana <mmahar...@cloudera.com>
Subject Re: New Impala Contributors: IMPALA-6296
Date Fri, 29 Dec 2017 19:12:31 GMT
I've pushed the initial changes to -
https://gerrit.cloudera.org/#/c/8923/

Steps that I've followed to make this contribution - As this is my first, I
want to elaborate a little bit to ensure I'm covering the bases right.

1. After the following the extensive contribution guide
<https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala>,
setup the development environment on machine with Ubuntu 16.04 OS, 15 GB
memory, Intel® Core™ i7-8550U CPU @ 1.80GHz × 8  & 512 GB SSD
2. Modified the build process to avoiding building against -so (IMPALA-5365
& IMPALA-3926)
3. Reproduced the issue. Attached Impalad error log.
4. Put in the code fix - added a condition check to ensure 'emit_perf_map_'
evaluates to true before the dependent object(perf_map_lock_) is asserted
using DCHECK.
5. Unit tested the changes using Impala-shell. No daemon crash encountered
this time.
6. Loaded and ran the "be" unit test cases to ensure the change did not
break other stuff. Attached the results.


Questions I have:
1. Is it recommened to run more end-to-end unit tests or does it depend on
the intensity of the change ? Like here I've limited it to just running be
tests, is that ok?
2. As per Tim's comment, I should see *.asm files in codegen-modules after
the fix, but I don't see any. Am I missing anything ?

Thanks & appreciate your input,
Mansi


On Fri, Dec 29, 2017 at 12:43 PM, Manaswini Maharana <mmaharana@cloudera.com
> wrote:

> Thank you!
>
> On Fri, Dec 29, 2017 at 11:47 AM, Jim Apple <jbapple@cloudera.com> wrote:
>
>> Done
>> On Fri, Dec 29, 2017 at 7:12 AM, Manaswini Maharana
>> <mmaharana@cloudera.com> wrote:
>> > username: mmaharana
>> >
>> > On Fri, Dec 29, 2017 at 10:07 AM, Jin Chul Kim <jinchul@gmail.com>
>> wrote:
>> >
>> >> Hi,
>> >>
>> >> I guess she doesn't have a privilege to change assignee. The permission
>> >> should be provided by somebody who has an admin privilege.
>> >> @Mansi, please share your username.
>> >>
>> >> Best regards,
>> >> Jinchul
>> >>
>> >> 2017-12-30 0:02 GMT+09:00 Vincent Tran <vttran@cloudera.com>:
>> >>
>> >> > You should be able to just simply "assign to me" if you are signed
>> into
>> >> ADD
>> >> > Jira.
>> >> >
>> >> > On Dec 29, 2017 9:54 AM, "Manaswini Maharana" <
>> mmaharana@cloudera.com>
>> >> > wrote:
>> >> >
>> >> > > Good morning, Team - any update on this?
>> >> > >
>> >> > > - Mansi
>> >> > >
>> >> > >
>> >> > > On Thu, Dec 28, 2017 at 12:32 AM, Manaswini Maharana <
>> >> > > mmaharana@cloudera.com
>> >> > > > wrote:
>> >> > >
>> >> > > > Hello team, I'd like to contribute to Impala and would like
to
>> start
>> >> it
>> >> > > > with this (IMPALA-6296) jira. Can I please have it assigned
to
>> me?
>> >> > > >
>> >> > > > My development environment is ready. I'm on ubuntu 16.04
so yes
>> some
>> >> > > > challenges to set up the environment especially with the
>> >> > LD_LIBRARY_PATH
>> >> > > > issues. So far I'm able to work past it and might ask for
another
>> >> pair
>> >> > of
>> >> > > > eyes to ensure it follows standard. The builds are going
fine and
>> >> runs
>> >> > a
>> >> > > > bit longer as I've disabled the -so. Apart from setup, I'm
was
>> able
>> >> to
>> >> > > > reproduce the issue and might have been ready with the fix
which
>> I'd
>> >> > like
>> >> > > > to discuss  once I'm officially assigned the jira.
>> >> > > >
>> >> > > > Thanks Tim, for the instructions, it was very helpful.
>> >> > > >
>> >> > > >
>> >> > > > Thanks!
>> >> > > > Mansi
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > > On Tue, Dec 12, 2017 at 8:24 PM, Tim Armstrong <
>> >> > tarmstrong@cloudera.com>
>> >> > > > wrote:
>> >> > > >
>> >> > > >> If you'd like to contribute a patch to Impala, but aren't
sure
>> what
>> >> > > >> you want to work on, you can look at Impala's newbie
issues:
>> >> > > >> https://issues.apache.org/jira/issues/?filter=12341668.
You can
>> >> find
>> >> > > >> detailed instructions on submitting patches at
>> >> > > >> https://cwiki.apache.org/confluence/display/IMPALA/
>> >> > > Contributing+to+Impala
>> >> > > >> .
>> >> > > >> This is a walkthrough of a ticket a new contributor could
take
>> on,
>> >> > > >> with hopefully enough detail to get you going but not
so much to
>> >> take
>> >> > > >> away the fun.
>> >> > > >>
>> >> > > >> How can we solve https://issues.apache.org/jira
>> /browse/IMPALA-6296,
>> >> > > >> "DCheck in CodegenSymbolEmitter when --asm_module_dir
is set on
>> >> debug
>> >> > > >> build
>> >> > > >> "?
>> >> > > >> First, make sure you have your development environment
set up.
>> Let's
>> >> > > >> see if we can reproduce the issue.
>> >> > > >>
>> >> > > >> Create a directory for the codegen modules then start
>> impala-server
>> >> > > >> $ mkdir codegen-modules
>> >> > > >> $ start-impala-cluster.py --impalad_args="--opt_module_d
>> >> > > >> ir=codegen-modules
>> >> > > >> --unopt_module_dir=codegen-modules --asm_module_dir=codegen-
>> >> modules"
>> >> > > >>
>> >> > > >> Now run a query through Impala. I chose this query because
it
>> uses
>> >> > > >> Impala's
>> >> > > >> runtime code generation ("codegen") to improve performance
>> >> > > >> $ bin/impala-shell.sh -q "select count(*) from tpch.lineitem"
>> >> > > >>
>> >> > > >> After a few seconds, you should see an error from Impala
>> crashing:
>> >> > > >> Error communicating with impalad: TSocket read 0 bytes
>> >> > > >> Could not execute command: select count(*) from tpch.lineitem
>> >> > > >>
>> >> > > >> Attempting to run any further queries will fail because
of the
>> >> crashed
>> >> > > >> Impala daemons:
>> >> > > >> $ impala-shell.sh -q "select count(*) from tpch.lineitem";
>> >> > > >> Starting Impala Shell without Kerberos authentication
>> >> > > >> Error connecting: TTransportException, Could not connect
to
>> >> > > >> localhost:21000
>> >> > > >> Not connected to Impala, could not execute queries.
>> >> > > >>
>> >> > > >> If you look at logs/cluster/impalad.ERROR you will see
a message
>> >> > > >> explaining
>> >> > > >> the crash - we hit a DCHECK (a.k.a. an assertion):
>> >> > > >>
>> >> > > >> F1212 17:10:07.642722  9138 codegen-symbol-emitter.cc:90]
Check
>> >> > failed:
>> >> > > >> perf_map_.find(obj.getData().data()) != perf_map_.end()
>> >> > > >> *** Check failure stack trace: ***
>> >> > > >>     @          0x3be16ed  google::LogMessage::Fail()
>> >> > > >>     @          0x3be2f92  google::LogMessage::SendToLog()
>> >> > > >>     @          0x3be10c7  google::LogMessage::Flush()
>> >> > > >>     @          0x3be468e  google::LogMessageFatal::~
>> >> LogMessageFatal()
>> >> > > >>     @          0x1b354f1
>> >> > > >> impala::CodegenSymbolEmitter::NotifyFreeingObject()
>> >> > > >>     @          0x375b791  llvm::MCJIT::NotifyFreeingObject()
>> >> > > >>     @          0x375b811  llvm::MCJIT::~MCJIT()
>> >> > > >>     @          0x375bfc9  llvm::MCJIT::~MCJIT()
>> >> > > >>     @          0x1b265ba  std::default_delete<>::operator()()
>> >> > > >>     @          0x1b23e21  std::unique_ptr<>::reset()
>> >> > > >>     @          0x1b11198  impala::LlvmCodeGen::Close()
>> >> > > >>     @          0x182194d  impala::RuntimeState::ReleaseR
>> esources()
>> >> > > >>     @          0x1893a35  impala::FragmentInstanceState:
>> :Close()
>> >> > > >>     @          0x1890d58  impala::FragmentInstanceState::Exec()
>> >> > > >>     @          0x1879afa  impala::QueryState::ExecFInstance()
>> >> > > >>     @          0x18783bc
>> >> > > >> _ZZN6impala10QueryState15StartFInstancesEvENKUlvE_clEv
>> >> > > >>     @          0x187a739
>> >> > > >> _ZN5boost6detail8function26void_function_obj_invoker0IZN6imp
>> >> > > >> ala10QueryState15StartFInstancesEvEUlvE_vE6invokeERNS1_
>> >> > > 15function_bufferE
>> >> > > >>     @          0x17c7200  boost::function0<>::operator()()
>> >> > > >>     @          0x1abdf8b  impala::Thread::SuperviseThread()
>> >> > > >>     @          0x1ac6b16  boost::_bi::list4<>::operator()<>()
>> >> > > >>     @          0x1ac6a59  boost::_bi::bind_t<>::operator()()
>> >> > > >>     @          0x1ac6a1c  boost::detail::thread_data<>::run()
>> >> > > >>     @          0x2d6b08a  thread_proxy
>> >> > > >>     @     0x7fcb5ff146ba  start_thread
>> >> > > >>     @     0x7fcb5fc4a3dd  clone
>> >> > > >>
>> >> > > >> It looks like something is wrong with the code - 'perf_map_'
>> doesn't
>> >> > > have
>> >> > > >> anything in it. If we look a few lines above, it looks
like
>> >> > 'perf_map_'
>> >> > > is
>> >> > > >> only filled in if 'emit_perf_map_' is true.
>> >> > > >>
>> >> > > >>   if (emit_perf_map_) {
>> >> > > >>     lock_guard<SpinLock> perf_map_lock(perf_map_lock_);
>> >> > > >>     DCHECK(perf_map_.find(obj.getData().data()) ==
>> >> perf_map_.end());
>> >> > > >>     perf_map_[obj.getData().data()] =
>> std::move(perf_map_entries);
>> >> > > >>     WritePerfMapLocked();
>> >> > > >>   }
>> >> > > >>
>> >> > > >> Maybe we should also be checking 'emit_perf_map_' before
the
>> DCHECK?
>> >> > > >>
>> >> > > >> After you have fixed this bug, you should be able to
run the
>> query
>> >> > > without
>> >> > > >> Impala crashing and you should be able to inspect the
assembly
>> >> > generated
>> >> > > >> by
>> >> > > >> Impala in codegen-modules/*.asm
>> >> > > >>
>> >> > > >> Have fun, and don't be afraid to ask dev@impala.apache.org
is
>> you
>> >> > have
>> >> > > >> any questions!
>> >> > > >>
>> >> > > >
>> >> > > >
>> >> > >
>> >> >
>> >>
>>
>
>

Mime
  • Unnamed multipart/mixed (inline, None, 0 bytes)
View raw message