TAJO-1950: Query master uses too much memory during range shuffle by jihoonson · Pull Request #884 · apache/tajo

jihoonson · 2015-11-27T07:01:29Z

No description provided.

jihoonson · 2015-11-27T07:03:26Z

This patch is not ready for review. I need more tests including CI test.

…into TAJO-1950

…into TAJO-1950 Conflicts: tajo-core/src/main/java/org/apache/tajo/worker/TaskImpl.java tajo-pullserver/src/main/java/org/apache/tajo/pullserver/TajoPullServerService.java

jihoonson · 2015-12-07T16:04:11Z

Hi, this patch is ready for review.
Here are highlights of this patch.

To reduce the number of fetches created in Repartitioner, I've merged fetches with their pull server host name. As a result, the total number of fetches is H * R, where H is the number of hosts and R is the number of ranges.
To reduce the size of fetches created in Repartitioner, I've changed to keep FetchProto instead of FetchImpl.
To reduce the number of index opens and closes, I've added an index reader cache to PullServerService.

jihoonson · 2015-12-09T08:26:00Z

I've additionally added some codes to clean up index cache upon eb completion.

jinossy · 2015-12-10T02:45:21Z

tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java

It should be future.channel().close()

jihoonson · 2015-12-10T06:36:18Z

@jinossy thanks for your review.
Actually, I found more problems related to the http message length. I'll fix it soon.

jinossy · 2015-12-10T07:34:36Z

I see, please go ahead.

jihoonson · 2015-12-14T06:08:55Z

@jinossy thanks! I've updated my patch.

jinossy · 2015-12-14T06:38:59Z

tajo-core/src/main/java/org/apache/tajo/worker/TaskImpl.java

@jihoonson
zero length chunk should be removed

…into TAJO-1950

jinossy · 2015-12-15T04:39:37Z

Looks good to me!
@hyunsik
Have you any comments?

…into TAJO-1950

hyunsik · 2015-12-16T06:23:37Z

I have a concern about too long of MAXIMUM HTTP REQUEST. It may cause too large granularity for repeat fetch.

jinossy · 2015-12-17T01:38:19Z

@hyunsik
You're right. it can cause read timeout.

@jihoonson
Could you change to configurable value ?

jihoonson · 2015-12-17T09:24:18Z

@hyunsik and @jinossy, thanks for your comment. I make the max url length configurable. The default value is 2 KB.

jinossy · 2015-12-18T03:27:00Z

+1 LGTM!

…TAJO-1950

jihoonson · 2015-12-21T09:29:36Z

@jinossy thanks for your review. I'm testing this patch against 10TB dataset. I'll share the result when it finishes and then commit this patch.

jihoonson · 2015-12-22T04:35:37Z

Well, there remain some issues around fetch timeout. When I tested this patch against 10TB dataset, a lot of fetch timeouts occurred while transferring intermediate data between stages. The main reason looks that index lookup takes a lot of time (over 30 seconds with cache miss). So, I think the fundamental solution is to improve index search performance which need to be handled in another jira.
So, I'd like to suggest to increase fetch timeout temporarily. With doubled timeout (that is 120 seconds), everything was ok against 10TB data.
@hyunsik, @jinossy what do you think?

…into TAJO-1950

jihoonson · 2015-12-24T08:46:05Z

I've decreased the default max url length to 1KB. Maybe we can find more optimized value of max url length and fetch timeout.

jinossy · 2015-12-24T09:10:24Z

@jihoonson
Thanks for your sharing. I guess, decreasing uri parameter will be better
Ship it!

jihoonson · 2015-12-24T09:16:28Z

@jinossy thanks for your comment. I'll commit shortly.
Merry christmas!

Closes apache#884

TAJO-1950

9345f0e

jihoonson changed the title ~~TAJO-1950: UniformRangePartition::partition() takes too long time when sorting large data~~ TAJO-1950: Query master uses too much memory during range shuffle Nov 27, 2015

jihoonson added 12 commits November 27, 2015 18:49

Reducing index open close

00944a6

Cache reader

2a9fc17

Fix NPE

0a6d8b0

Reduce log

0f7814b

test

a20b912

TAJO-1950

6d24918

fix test failure

4e4f9b7

reduce log

fbbf7b2

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo …

bae6540

…into TAJO-1950

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo …

967bb5e

…into TAJO-1950 Conflicts: tajo-core/src/main/java/org/apache/tajo/worker/TaskImpl.java tajo-pullserver/src/main/java/org/apache/tajo/pullserver/TajoPullServerService.java

Fix bug

6f2bce1

Fix test failure

63e7062

jihoonson added 5 commits December 8, 2015 10:29

improve logging

40839b5

TAJO-1950

31012d7

Add reference count to index

6136d74

Cleanup cache after eb

bc5288b

Clean up cache whenever an eb is finished

e00f85c

jihoonson added 2 commits December 9, 2015 17:59

Fix NPE at TestHA

9cc958a

TAJO-1950

9ed08ca

jinossy reviewed Dec 10, 2015
View reviewed changes

tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java Outdated

Copy link

Member

jinossy Dec 10, 2015

Choose a reason for hiding this comment

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

It should be future.channel().close()

jihoonson added 3 commits December 10, 2015 17:16

Restore max url length

772c0e3

Adding log

172b5cd

Add more logs

be1a16c

Reflect comments

d0e9bf2

jinossy reviewed Dec 14, 2015
View reviewed changes

tajo-core/src/main/java/org/apache/tajo/worker/TaskImpl.java

Copy link

Member

jinossy Dec 14, 2015

Choose a reason for hiding this comment

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

@jihoonson
zero length chunk should be removed

jihoonson added 3 commits December 14, 2015 16:01

Check file length of remote chunks

15d569e

trigger ci

29e0050

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo …

6358cb2

…into TAJO-1950

jihoonson added 2 commits December 16, 2015 12:24

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo …

08b4994

…into TAJO-1950

Fix compilation error in task.jsp

f2ff35c

jihoonson added 5 commits December 16, 2015 15:57

Decrease max url length

ffcd614

reduce header size

a5fb935

move bootstrap handler initialization

2c086e4

fix test failure

c1cbfef

Decrease max url length

1f4124a

Make max url length configurable

04dd9ee

jihoonson added 2 commits December 20, 2015 07:42

Add log

04a335b

Merge branch 'TAJO-1950' of https://github.com/jihoonson/tajo-2 into …

d4d7781

…TAJO-1950

jihoonson added 2 commits December 24, 2015 16:58

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo …

f05ffca

…into TAJO-1950

decrease max url length to 1K

80b1205

asfgit closed this in 1f9ae1d Dec 24, 2015

combineads pushed a commit to combineads/tajo that referenced this pull request Dec 29, 2015

TAJO-1950: Query master uses too much memory during range shuffle.

70c5e64

Closes apache#884

Conversation

jihoonson commented Nov 27, 2015

Uh oh!

jihoonson commented Nov 27, 2015

Uh oh!

jihoonson commented Dec 7, 2015

Uh oh!

jihoonson commented Dec 9, 2015

Uh oh!

jinossy Dec 10, 2015

Choose a reason for hiding this comment

Uh oh!

jihoonson commented Dec 10, 2015

Uh oh!

jinossy commented Dec 10, 2015

Uh oh!

jihoonson commented Dec 14, 2015

Uh oh!

jinossy Dec 14, 2015

Choose a reason for hiding this comment

Uh oh!

jinossy commented Dec 15, 2015

Uh oh!

hyunsik commented Dec 16, 2015

Uh oh!

jinossy commented Dec 17, 2015

Uh oh!

jihoonson commented Dec 17, 2015

Uh oh!

jinossy commented Dec 18, 2015

Uh oh!

jihoonson commented Dec 21, 2015

Uh oh!

jihoonson commented Dec 22, 2015

Uh oh!

jihoonson commented Dec 24, 2015

Uh oh!

jinossy commented Dec 24, 2015

Uh oh!

jihoonson commented Dec 24, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants