Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-928: Session variables should override query configs in TajoConf.#98

Closed
hyunsik wants to merge 22 commits into
apache:masterfrom
hyunsik:TAJO-928
Closed

TAJO-928: Session variables should override query configs in TajoConf.#98
hyunsik wants to merge 22 commits into
apache:masterfrom
hyunsik:TAJO-928

Conversation

@hyunsik
Copy link
Copy Markdown
Member

@hyunsik hyunsik commented Jul 30, 2014

No description provided.

hyunsik added 17 commits July 8, 2014 17:47
…into OUTPUT_ROTATING

Conflicts:
	tajo-core/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java
	tajo-core/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java
	tajo-core/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
…into OUTPUT_ROTATING

Conflicts:
	tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/HashBasedColPartitionStoreExec.java
	tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortBasedColPartitionStoreExec.java
…into TAJO-928

Conflicts:
	tajo-core/src/main/java/org/apache/tajo/master/querymaster/Repartitioner.java
@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Jul 30, 2014

This is still ongoing work. The remain works are as follows:

  • Some session variables only used in cli will be removed
  • Add more comments and description
  • Add new meta comment for tsql display configuration
  • Add documentation about new session variable system

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 3, 2014

It is ready to be reviewed. Please review the patch. I'll also add some documentation soon.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the commented out lines.

hyunsik added 2 commits August 6, 2014 19:02
…into TAJO-928

Conflicts:
	tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 7, 2014

I've described some changes.

The current way is to set some keys in TajoConf.ConfVars. I deprecated the way, but the way still works for a while. If you use the old way, you can see the deprecation warning.

Try \? for help.
default> \set tajo.dist-query.join.broadcast.threshold-bytes 50000
Warning: deprecated to directly use config key in TajoConf.ConfVars. Please execute '\help set'.

But, the conf key automatically is replaced by new key corresponding to the conf key.

default> \set
'BROADCAST_TABLE_SIZE_LIMIT'='50000'
'CURRENT_DATABASE'='default'
'SESSION_ID'='1a104a50-3f8d-4bac-a8a2-5a8efc936298'
'USERNAME'='hyunsik'
'SESSION_LAST_ACCESS_TIME'='1407388966573'

Also, I've added the help command and its alias. So you can use \? [command name] or \help [command name]. I've extended some method for help to TajoShellCommand interface. The following is the help of set command.

default> \help set

Available Session Variables:

\set SESSION_EXPIRY_TIME [int value] - session expiry time (secs)
\set CLI_COLUMNS [int value] - Sets the width for the wrapped format
\set CLI_FORMATTER_CLASS [text value] - Sets the output format class to display results
\set CLI_NULL_CHAR [text value] - Sets the string to be printed in place of a null value.
\set CLI_PAGE_ROWS [int value] - Sets the number of rows for paging
\set CLI_PAGING_ENABLED [true or false] - Enable paging of result display
\set CLI_DISPLAY_ERROR_TRACE [true or false] - Enable display of error trace
\set ON_ERROR_STOP [true or false] - tsql will exist if an error occurs.
\set TZ [text value] - Sets timezone
\set DATE_ORDER [text value] - date order (default is YMD)
\set LANG [text value] - Language
\set LC_ALL [text value] - String sort order
\set LC_COLLATE [text value] - String sort order
\set LC_CTYPE [text value] - Character classification (What is a letter? Its upper-case equivalent?)
\set LC_MESSAGES [text value] - Language of messages
\set LC_MONETARY [text value] - Formatting of currency amounts
\set LC_NUMERIC [text value] - Formatting of numbers
\set LC_TIME [text value] - Formatting of dates and times
\set BROADCAST_TABLE_SIZE_LIMIT [long value] - limited size (bytes) of broadcast table
\set JOIN_TASK_INPUT_SIZE [int value] - join task input size (mb) 
\set SORT_TASK_INPUT_SIZE [int value] - sort task input size (mb)
\set GROUPBY_TASK_INPUT_SIZE [int value] - group by task input size (mb)
\set JOIN_PER_SHUFFLE_SIZE [int value] - shuffle output size for join (mb)
\set GROUPBY_PER_SHUFFLE_SIZE [int value] - shuffle output size for sort (mb)
\set TABLE_PARTITION_PER_SHUFFLE_SIZE [int value] - shuffle output size for partition table write (mb)
\set EXTSORT_BUFFER_SIZE [long value] - sort buffer size for external sort (mb)
\set HASH_JOIN_SIZE_LIMIT [long value] - limited size for hash join (mb)
\set INNER_HASH_JOIN_SIZE_LIMIT [long value] - limited size for hash inner join (mb)
\set OUTER_HASH_JOIN_SIZE_LIMIT [long value] - limited size for hash outer join (mb)
\set HASH_GROUPBY_SIZE_LIMIT [long value] - limited size for hash groupby (mb)
\set MAX_OUTPUT_FILE_SIZE [int value] - Maximum per-output file size (mb). 0 means infinite.
\set NULL_CHAR [text value] - null char of text file output
\set ARITHABORT [true or false] - If true, a running query will be terminated when an overflow or divide-by-zero occurs.
\set DEBUG_ENABLED [true or false] - (debug only) debug mode enabled

Also, I've updated querydetail.jsp file to show session variables applied to the query.

I believe that this patch is ready to be committed. Please review this.

@blrunner
Copy link
Copy Markdown
Contributor

blrunner commented Aug 7, 2014

Thank you for your detailed description and sorry but I'm hard to review it now.
Could I review it tonight?

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 7, 2014

@blrunner Thanks in advance!

@blrunner
Copy link
Copy Markdown
Contributor

blrunner commented Aug 8, 2014

Hi @hyunsik

Unfortunately, 'mvn clean install' failed as follows:
{code:xml}
Results :

Failed tests: testForwardedQuery(org.apache.tajo.scheduler.TestFifoScheduler): expected:<QUERY_RUNNING> but was:<QUERY_NOT_ASSIGNED>

Tests run: 1096, Failures: 1, Errors: 0, Skipped: 0
{code}

Could you check it?

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 8, 2014

I haven't experienced the failure. The failure seems to occur occasionally due to unknown reason. In addition, I strongly believe that this change affects the test.

@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 8, 2014

If the failure occurs occasionally, we need to handle it in another jira issue.

@blrunner
Copy link
Copy Markdown
Contributor

Sorry for my late review.

Could you rebase against the master branch?
I failed to merge it as follows:
{code:xml}
CONFLICT (content): Merge conflict in tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java
Auto-merging tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java
Automatic merge failed; fix conflicts and then commit the result.
{code}

…into TAJO-928

Conflicts:
	tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java
@hyunsik
Copy link
Copy Markdown
Member Author

hyunsik commented Aug 11, 2014

I've rebased the patch against the latest revision. Thank you for your review.

@blrunner
Copy link
Copy Markdown
Contributor

+1

Thank you for your great patch, it will be very useful to tajo.
It looks good overall and I tested it on my testing cluster.
For reference, It works expectedly.

@asfgit asfgit closed this in ddfc3f3 Aug 11, 2014
@hyunsik hyunsik deleted the TAJO-928 branch August 11, 2014 03:57
babokim pushed a commit to babokim/tajo that referenced this pull request Dec 11, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants