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 Wed, 03 Jan 2018 18:06:32 GMT
Thanks for the pointers, Thomas. I was able to run the Jenkin tests, but
not sure how to manually add the links to Gerrit as recommended. Do I  add
it through the reply text box as shown below?

- Mansi
​

On Tue, Jan 2, 2018 at 3:04 PM, Thomas Tauber-Marshall <
tmarshall@cloudera.com> wrote:

> On Fri, Dec 29, 2017 at 11:12 AM Manaswini Maharana <
> mmaharana@cloudera.com>
> wrote:
>
> > 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/Contribu
> ting+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?
> >
>
> In general, you're expected to run any tests that may reasonably be
> affected by the change, and of course the more testing the better. This
> change affects basically any query that uses codegen, which is a
> significant portion of our tests, so running the full suite would probably
> be a good idea (eg bin/run-all-tests.sh)
>
> You may also want to try out the Jenkins jobs that we provide for testing
> patches, as described here:
> https://cwiki.apache.org/confluence/display/IMPALA/Using+
> Gerrit+to+submit+and+review+patches#UsingGerrittosubmitand
> reviewpatches-Verifyingapatch(opentoallImpalacontributors)
>
>
> > 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 ?
> >
>
> You may need to manually create the directory $IMPALA_HOME/codegen-modules/
>
>
> >
> > 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<>::operato
> r()()
> >>> >> > > >>     @          0x1b23e21  std::unique_ptr<>::reset()
> >>> >> > > >>     @          0x1b11198  impala::LlvmCodeGen::Close()
> >>> >> > > >>     @          0x182194d
> >>> impala::RuntimeState::ReleaseResources()
> >>> >> > > >>     @          0x1893a35
> >>> impala::FragmentInstanceState::Close()
> >>> >> > > >>     @          0x1890d58  impala::FragmentInstanceState:
> :Exec()
> >>> >> > > >>     @          0x1879afa  impala::QueryState::ExecFInsta
> nce()
> >>> >> > > >>     @          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/related (inline, None, 0 bytes)
View raw message