[Impala-ASF-CR] IMPALA-3784: Fixed borken status-benchmark on debug

2017-11-23 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8641


Change subject: IMPALA-3784: Fixed borken status-benchmark on debug
..

IMPALA-3784: Fixed borken status-benchmark on debug

A default constructed boost::optional is empty - it does
not contain a value, so get() cannot be called. The value
check should be required before using it.

Change-Id: I2fcdbdb4c5bc432a4d224dda821de132e3bca611
---
M be/src/benchmarks/status-benchmark.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/8641/1
--
To view, visit http://gerrit.cloudera.org:8080/8641
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2fcdbdb4c5bc432a4d224dda821de132e3bca611
Gerrit-Change-Number: 8641
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul 


[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.

2017-11-23 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8611 )

Change subject: IMPALA-6225: Query profile date-time strings should have ns 
precision.
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8611/7/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/8611/7/tests/query_test/test_observability.py@133
PS7, Line 133: while (retries < 120)
> GVO failed because 120s doesn't seem to be a good enough timeout.
I tried running this test alone locally and it appears to hang every now and 
then. So the timeout may be red-herring. There seems to be something wrong.



--
To view, visit http://gerrit.cloudera.org:8080/8611
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0
Gerrit-Change-Number: 8611
Gerrit-PatchSet: 7
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 23 Nov 2017 20:01:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4664: Unexpected string conversion in Shell

2017-11-23 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8639 )

Change subject: IMPALA-4664: Unexpected string conversion in Shell
..

IMPALA-4664: Unexpected string conversion in Shell

Impala shell can accidentally convert certain
literal strings to lowercase. Impala shell splits
each command into tokens and then converts the
first token to lowercase to figure out how it
should execute the command. The splitting is done
by spaces only. Thus, if the user types a TAB
after the SELECT, the first token after the split
becomes the SELECT plus whatever comes after it.

Using sqlparse library in infra instead of
shell/ext-py/sqlparse-0.1.14. The sqlparse
library in infra is upgraded from 0.1.15 to
0.2.4 due to bug fixes and syntax extension.

Testing:
Add tests to tests/shell/test_shell_interactive.py

Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
---
M LICENSE.txt
M README.md
M bin/rat_exclude_files.txt
M infra/python/deps/requirements.txt
M shell/.gitignore
D shell/ext-py/sqlparse-0.1.14/AUTHORS
D shell/ext-py/sqlparse-0.1.14/CHANGES
D shell/ext-py/sqlparse-0.1.14/COPYING
D shell/ext-py/sqlparse-0.1.14/MANIFEST.in
D shell/ext-py/sqlparse-0.1.14/PKG-INFO
D shell/ext-py/sqlparse-0.1.14/README.rst
D shell/ext-py/sqlparse-0.1.14/TODO
D shell/ext-py/sqlparse-0.1.14/bin/sqlformat
D shell/ext-py/sqlparse-0.1.14/docs/source/analyzing.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/api.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/changes.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/conf.py
D shell/ext-py/sqlparse-0.1.14/docs/source/index.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/indices.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/intro.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/ui.rst
D shell/ext-py/sqlparse-0.1.14/docs/sqlformat.1
D shell/ext-py/sqlparse-0.1.14/pytest.ini
D shell/ext-py/sqlparse-0.1.14/setup.cfg
D shell/ext-py/sqlparse-0.1.14/setup.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/__init__.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/__init__.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/filter.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/grouping.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/exceptions.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/filters.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/formatter.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/functions.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/keywords.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/pipeline.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/sql.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/tokens.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/utils.py
D shell/ext-py/sqlparse-0.1.14/tests/__init__.py
D shell/ext-py/sqlparse-0.1.14/tests/files/_Make_DirEntry.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/begintag.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/begintag_2.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/dashcomment.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql2.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql3.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/huge_select.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/test_cp1251.sql
D shell/ext-py/sqlparse-0.1.14/tests/test_filters.py
D shell/ext-py/sqlparse-0.1.14/tests/test_format.py
D shell/ext-py/sqlparse-0.1.14/tests/test_functions.py
D shell/ext-py/sqlparse-0.1.14/tests/test_grouping.py
D shell/ext-py/sqlparse-0.1.14/tests/test_parse.py
D shell/ext-py/sqlparse-0.1.14/tests/test_pipeline.py
D shell/ext-py/sqlparse-0.1.14/tests/test_regressions.py
D shell/ext-py/sqlparse-0.1.14/tests/test_split.py
D shell/ext-py/sqlparse-0.1.14/tests/test_tokenize.py
D shell/ext-py/sqlparse-0.1.14/tests/utils.py
D shell/ext-py/sqlparse-0.1.14/tox.ini
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
63 files changed, 36 insertions(+), 6,715 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8639/2
--
To view, visit http://gerrit.cloudera.org:8080/8639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
Gerrit-Change-Number: 8639
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Kim Jin Chul 


[Impala-ASF-CR] IMPALA-4664: Unexpected string conversion in Shell

2017-11-23 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8639 )

Change subject: IMPALA-4664: Unexpected string conversion in Shell
..


Patch Set 1:

shell/ext-py/sqlparse-0.1.14 includes only one fix for escaping backslashes. 
The fix was delivered to upstream by this change: 
https://github.com/andialbrecht/sqlparse/commit/e75e35869473832a1eb67772b1adfee2db11b85a

Therefore, we don't have to keep this library on our repository. The version 
0.1.14 is too old which was released in Nov 30, 2014. I would recommend to use 
recent and stable version 0.2.4. The old version has bugs for parsing such as 
following cases:
* Variable assignment. The problem is that the tokens '=' '5' disappear.
(e.g. set mt_dop=5;)
* A series of queires in a single row.
(e.g. select'DIG';select'DIG';)

Please see the changelog of sqlparse: 
https://github.com/andialbrecht/sqlparse/blob/master/CHANGELOG


--
To view, visit http://gerrit.cloudera.org:8080/8639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
Gerrit-Change-Number: 8639
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Thu, 23 Nov 2017 16:48:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4664: Unexpected string conversion in Shell

2017-11-23 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8639


Change subject: IMPALA-4664: Unexpected string conversion in Shell
..

IMPALA-4664: Unexpected string conversion in Shell

Impala shell can accidentally convert certain
literal strings to lowercase. Impala shell splits
each command into tokens and then converts the
first token to lowercase to figure out how it
should execute the command. The splitting is done
by spaces only. Thus, if the user types a TAB
after the SELECT, the first token after the split
becomes the SELECT plus whatever comes after it.

Using sqlparse library in infra instead of
shell/ext-py/sqlparse-0.1.14. The sqlparse
library in infra is upgraded from 0.1.15 to
0.2.4 due to bug fixes and syntax extension.

Testing:
Add tests to tests/shell/test_shell_interactive.py

Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
---
M LICENSE.txt
M README.md
M bin/rat_exclude_files.txt
M infra/python/deps/requirements.txt
M shell/.gitignore
D shell/ext-py/sqlparse-0.1.14/AUTHORS
D shell/ext-py/sqlparse-0.1.14/CHANGES
D shell/ext-py/sqlparse-0.1.14/COPYING
D shell/ext-py/sqlparse-0.1.14/MANIFEST.in
D shell/ext-py/sqlparse-0.1.14/PKG-INFO
D shell/ext-py/sqlparse-0.1.14/README.rst
D shell/ext-py/sqlparse-0.1.14/TODO
D shell/ext-py/sqlparse-0.1.14/bin/sqlformat
D shell/ext-py/sqlparse-0.1.14/docs/source/analyzing.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/api.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/changes.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/conf.py
D shell/ext-py/sqlparse-0.1.14/docs/source/index.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/indices.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/intro.rst
D shell/ext-py/sqlparse-0.1.14/docs/source/ui.rst
D shell/ext-py/sqlparse-0.1.14/docs/sqlformat.1
D shell/ext-py/sqlparse-0.1.14/pytest.ini
D shell/ext-py/sqlparse-0.1.14/setup.cfg
D shell/ext-py/sqlparse-0.1.14/setup.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/__init__.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/__init__.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/filter.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/engine/grouping.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/exceptions.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/filters.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/formatter.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/functions.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/keywords.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/pipeline.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/sql.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/tokens.py
D shell/ext-py/sqlparse-0.1.14/sqlparse/utils.py
D shell/ext-py/sqlparse-0.1.14/tests/__init__.py
D shell/ext-py/sqlparse-0.1.14/tests/files/_Make_DirEntry.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/begintag.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/begintag_2.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/dashcomment.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql2.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/function_psql3.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/huge_select.sql
D shell/ext-py/sqlparse-0.1.14/tests/files/test_cp1251.sql
D shell/ext-py/sqlparse-0.1.14/tests/test_filters.py
D shell/ext-py/sqlparse-0.1.14/tests/test_format.py
D shell/ext-py/sqlparse-0.1.14/tests/test_functions.py
D shell/ext-py/sqlparse-0.1.14/tests/test_grouping.py
D shell/ext-py/sqlparse-0.1.14/tests/test_parse.py
D shell/ext-py/sqlparse-0.1.14/tests/test_pipeline.py
D shell/ext-py/sqlparse-0.1.14/tests/test_regressions.py
D shell/ext-py/sqlparse-0.1.14/tests/test_split.py
D shell/ext-py/sqlparse-0.1.14/tests/test_tokenize.py
D shell/ext-py/sqlparse-0.1.14/tests/utils.py
D shell/ext-py/sqlparse-0.1.14/tox.ini
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
63 files changed, 37 insertions(+), 6,715 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8639/1
--
To view, visit http://gerrit.cloudera.org:8080/8639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
Gerrit-Change-Number: 8639
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul 


[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables

2017-11-23 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8621 )

Change subject: IMPALA-3703: Store query context in thread-local variables
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h
File be/src/common/thread-info.h:

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@61
PS2, Line 61: }
> The buffers were initialized to null by the constructor's init-list. Now th
When I said initialized to null, I meant they were filled with zeros.



--
To view, visit http://gerrit.cloudera.org:8080/8621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Gerrit-Change-Number: 8621
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 23 Nov 2017 12:29:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables

2017-11-23 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8621 )

Change subject: IMPALA-3703: Store query context in thread-local variables
..


Patch Set 4:

(13 comments)

Thanks all!

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info-test.cc
File be/src/common/thread-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info-test.cc@55
PS2, Line 55: info_table.AddExtraInfo("extra", "info");
> I agree it would be helpful to have some more comments. Maybe at least a co
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info-test.cc@55
PS2, Line 55: info_table.AddExtraInfo("extra", "info");
> please add a comment about what's going on:
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h
File be/src/common/thread-info.h:

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@37
PS2, Line 37: /// function 'GetThreadInfoTable()'.
> "Until this object lives" -> "While this object is alive"
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@41
PS2, Line 41: public:
> This is the only usage of boost:noncopyable I've seen in our code base. We
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@41
PS2, Line 41: public:
> Yeah DISALLOW_COPY_AND_ASSIGN is the canonical way to do it in Impala (for
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@42
PS2, Line 42:   // Only one InfoTable object can be alive per thread at a time.
> We've generally been moving to using the new inline initialisation syntax f
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@43
PS2, Line 43:   InfoTable() {
> We should comment that you're only allowed to create one of these per threa
Yeah it is a bit twisted because we need to create this object on the stack 
(for minidumps), then make the global pointer point to it.
I added a comment about it.


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@61
PS2, Line 61: }
> Would line.copy(text + text_size, line.length()) be clearer here?
The buffers were initialized to null by the constructor's init-list. Now they 
are initialized to null by the inline member initializers.

I switched to the member copy.


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@101
PS2, Line 101: dInfo(
> We generally use int64_t for byte sizes in the codebase. There are argument
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@105
PS2, Line 105:   static constexpr int64_t THREAD_NAME_SIZE = 256;
> The naming convention for member variables is to include a _ suffix. In thi
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@108
PS2, Line 108:   int64_t text_size_ = 0;
> You're choosing to store this as a string intentionally?
I thought this makes easier to display it during a debug session.
Also, if we want to use it for prepending log lines, it might be more efficient 
to have it as a string already.
But I can switch to TUniqueId if needed.


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/common/thread-info.h@115
PS2, Line 115: };
> By convention we generally use pointers if arguments or return values are m
Done


http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

http://gerrit.cloudera.org:8080/#/c/8621/2/be/src/runtime/query-state.cc@378
PS2, Line 378:   thread_info_table->SetQueryId(fis->query_id());
> We should also do this for other threads running in the context of the quer
For starter, I fill the InfoTable on these callsites.

I am thinking about extending the Thread class, making it aware of InfoTables. 
Like a child thread's InfoTable is the copy of the parent InfoTable at the 
beginning.
Or, I extend the InfoTable to have a pointer to the parent's InfoTable. This 
way we would have a tree-like structure of how the threads created each other. 
This works if threads always join, a bit trickier if threads can be detached.

What do you think about it?



--
To view, visit http://gerrit.cloudera.org:8080/8621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Gerrit-Change-Number: 8621
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 23 Nov 2017 12:26:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables

2017-11-23 Thread Zoltan Borok-Nagy (Code Review)
Hello Lars Volker, Philip Zeyliger, Tim Armstrong, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8621

to look at the new patch set (#4).

Change subject: IMPALA-3703: Store query context in thread-local variables
..

IMPALA-3703: Store query context in thread-local variables

This commit introduces the InfoTable class which can hold
information about the current thread that can be useful
in debug sessions.
It needs to be allocated on the stack in order to include
it to minidumps.

Currently an InfoTable object is created in
Thread::SuperviseThread. This object is available in all
the child stack frames through the global function
GetThreadInfoTable().

InfoTable has members for the thread name, query id, and
instance id. These are fixed size char buffers.
InfoTable also has a member called text, which is a char buffer
that holds lines of strings. A line has a format of
 = . New data can be appended to the buffer by
calling AddExtraInfo(key, value).

If you have a fully-fledged core file, you can locate the
InfoTable for the current thread through the global pointer
impala::thread_info_table.

In a core file that has been created from a minidump,
we need to select the stack frame that allocated the InfoTable
object in order to inspect it. It is currently allocated in
Thread::SuperviseThread().

We can use printf in gdb to print the members, e.g.:
printf "%s" thread_info_table.text
printf "%s\n" thread_info_table.query_id

Currently the thread category, thread name, query id, and
instance id is stored.

I created some tests in thread-info-test.cc

Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
---
M be/src/common/CMakeLists.txt
A be/src/common/thread-info-test.cc
A be/src/common/thread-info.cc
A be/src/common/thread-info.h
M be/src/exec/blocking-join-node.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/query-state.cc
M be/src/util/thread.cc
9 files changed, 322 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/8621/4
--
To view, visit http://gerrit.cloudera.org:8080/8621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Gerrit-Change-Number: 8621
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables

2017-11-23 Thread Zoltan Borok-Nagy (Code Review)
Hello Lars Volker, Philip Zeyliger, Tim Armstrong, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8621

to look at the new patch set (#3).

Change subject: IMPALA-3703: Store query context in thread-local variables
..

IMPALA-3703: Store query context in thread-local variables

This commit introduces the InfoTable class which can hold
information about the current thread that can be useful
in debug sessions.
It needs to be allocated on the stack in order to include
it to minidumps.

Currently an InfoTable object is created in
Thread::SuperviseThread. This object is available in all
the child stack frames through the global function
GetThreadInfoTable().

InfoTable has members for the thread name, query id, and
instance id. These are fixed size char buffers.
InfoTable also has a member called text, which is a char buffer
that holds lines of strings. A line has a format of
 = . New data can be appended to the buffer by
calling AddExtraInfo(key, value).

If you have a fully-fledged core file, you can locate the
InfoTable for the current thread through the global pointer
impala::thread_info_table.

In a core file that has been created from a minidump,
we need to select the stack frame that allocated the InfoTable
object in order to inspect it. It is currently allocated in
Thread::SuperviseThread().

We can use printf in gdb to print the members, e.g.:
printf "%s" thread_info_table.text
printf "%s\n" thread_info_table.query_id

Currently the thread category, thread name, query id, and
instance id is stored.

I created some tests in thread-info-test.cc

Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
---
M be/src/common/CMakeLists.txt
A be/src/common/thread-info-test.cc
A be/src/common/thread-info.cc
A be/src/common/thread-info.h
M be/src/runtime/query-state.cc
M be/src/util/thread.cc
6 files changed, 303 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/8621/3
--
To view, visit http://gerrit.cloudera.org:8080/8621
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Gerrit-Change-Number: 8621
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy