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 <mmahar...@cloudera.com > wrote: > Thank you! > > On Fri, Dec 29, 2017 at 11:47 AM, Jim Apple <jbap...@cloudera.com> wrote: > >> Done >> On Fri, Dec 29, 2017 at 7:12 AM, Manaswini Maharana >> <mmahar...@cloudera.com> wrote: >> > username: mmaharana >> > >> > On Fri, Dec 29, 2017 at 10:07 AM, Jin Chul Kim <jinc...@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 <vtt...@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" < >> mmahar...@cloudera.com> >> >> > wrote: >> >> > >> >> > > Good morning, Team - any update on this? >> >> > > >> >> > > - Mansi >> >> > > >> >> > > >> >> > > On Thu, Dec 28, 2017 at 12:32 AM, Manaswini Maharana < >> >> > > mmahar...@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 < >> >> > tarmstr...@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! >> >> > > >> >> >> > > > >> >> > > > >> >> > > >> >> > >> >> >> > >
