You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by jinossy <gi...@git.apache.org> on 2014/07/31 09:22:44 UTC
[GitHub] tajo pull request: TAJO-985: Client API should be async
GitHub user jinossy opened a pull request:
https://github.com/apache/tajo/pull/99
TAJO-985: Client API should be async
The query status synchronization on state machine always wait for time (event processing time) before status changes. If a lot of time are being processed to the event dispatcher, client api(getStatus, getProgress) will be waiting for changed event status. and query will be session timed out.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jinossy/tajo TAJO-985
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/99.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #99
----
commit 565744dea7ebcd80de100741465cab606fdad068
Author: jinossy <ji...@gmail.com>
Date: 2014-07-29T08:54:00Z
TAJO-985: Client API should be async
commit e4eb0fe704a7499858ba4821910e75f1577c79a8
Author: jinossy <ji...@gmail.com>
Date: 2014-07-30T02:50:58Z
fixed wrong progress
commit e6326d8b6dd26742de026dba1d0a4d34f2bc99cd
Author: jinossy <ji...@gmail.com>
Date: 2014-07-30T02:57:20Z
applied to web ui
commit 59425a86fae13afb90cda6c91fbc2ce02bfdc8ed
Author: jinossy <ji...@gmail.com>
Date: 2014-07-31T07:01:43Z
added event executor
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be non-blocking
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy closed the pull request at:
https://github.com/apache/tajo/pull/99
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/99#discussion_r15741545
--- Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java ---
@@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) {
while (true) {
// TODO - configurable
status = client.getQueryStatus(queryId);
- if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
+ if(status.getState() == QueryState.QUERY_NEW || status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
--- End diff --
The line is longer than 120 columns.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-51736398
The main idea looks good to me. It will mitigate lock contention problem.
I have one suggestion. How about changing getState(boolean) and getState signatures into different names? Since the uses of them may be sensitive, we need to know what we invoke more apparently.
I would like to suggest getSynchronizedState() and getState(). It's just a suggestion. The decision is up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-51149887
Thank you for the review. I reflected your comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-52447530
The word 'async' was changed during the review. So, I think that the issue title also should be changed to proper name. Could you change the title before committing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/99#discussion_r15741540
--- Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java ---
@@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) {
while (true) {
// TODO - configurable
status = client.getQueryStatus(queryId);
- if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
--- End diff --
The line is longer than 120 columns.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-51748431
I’ve reflects your suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be non-blocking
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-52450810
Thank you for the review.
I've just committed it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-52447449
+1
The patch looks nice to me. Ship it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/99#discussion_r16035452
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/querymaster/Query.java ---
@@ -337,15 +340,24 @@ public SubQuery getSubQuery(ExecutionBlockId id) {
return this.subqueries.values();
}
- public QueryState getState() {
- readLock.lock();
- try {
- return stateMachine.getCurrentState();
- } finally {
- readLock.unlock();
+ protected QueryState getState(boolean async) {
--- End diff --
I think that the variable name ```async``` should be nonblocking.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-985: Client API should be async
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/99#issuecomment-51737829
@hyunsik good idea.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---