This repository has been archived by the owner on Nov 3, 2023. It is now read-only.
cherry picks #56
Merged
cherry picks #56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit 5c106b9)
(cherry picked from commit 0509d7a)
By stop polling when presto query already finished.
When user make queries to Presto via SQL Lab, presto will run the query
and then it can return all data back to superset in one shot.
However, the default implementation of superset has enabled a default
polling for presto to:
- Get the fancy progress bar
- Get the data back when the query finished.
However, the polling implementation of superset is not right.
I've done a profiling with a table of 1 billion rows, here're some data:
- Total number of rows: 1.02 Billion
- SQL Lab query limit: 1 million
- Output Data: 1.5 GB
- Superset memory consumed: about 10-20 GB
- Time: 7 minutes to finish in Presto, takes additional 15 minutes for
superset to get and store data.
The problems with default issue is, even if presto has finished the
query (7 minutes with above profiling), superset still do lots of wasted
polling, in above profiling, superset sent about 540 polling in total,
and at half of the polling is not necessary.
Part of the simplied polling response:
```
{
"infoUri": "http://10.65.204.39:8000/query.html?20180525_042715_03742_nza9u",
"id": "20180525_042715_03742_nza9u",
"nextUri": "http://10.65.204.39:8000/v1/statement/20180525_042715_03742_nza9u/11",
"stats": {
"state": "FINISHED",
"queuedSplits": 21701,
"progressPercentage": 35.98264191882267,
"elapsedTimeMillis": 1029,
"nodes": 116,
"completedSplits": 15257,
"scheduled": true,
"wallTimeMillis": 2571904,
"peakMemoryBytes": 0,
"processedBytes": 40825519532,
"processedRows": 47734066,
"queuedTimeMillis": 0,
"queued": false,
"cpuTimeMillis": 849228,
"rootStage": {
"state": "FINISHED",
"queuedSplits": 0,
"nodes": 1,
"totalSplits": 17,
"processedBytes": 16829644,
"processedRows": 11495,
"completedSplits": 17,
"stageId": "0",
"done": true,
"cpuTimeMillis": 69,
"subStages": [
{
"state": "CANCELED",
"queuedSplits": 21701,
"nodes": 116,
"totalSplits": 42384,
"processedBytes": 40825519532,
"processedRows": 47734066,
"completedSplits": 15240,
"stageId": "1",
"done": true,
"cpuTimeMillis": 849159,
"subStages": [],
"wallTimeMillis": 2570374,
"userTimeMillis": 730020,
"runningSplits": 5443
}
],
"wallTimeMillis": 1530,
"userTimeMillis": 50,
"runningSplits": 0
},
"totalSplits": 42401,
"userTimeMillis": 730070,
"runningSplits": 5443
}
}
}
```
Superset will terminate the polling when it finds that `nextUri`
becomes none, but actually, when `["stats"]["state"] == "FINISHED"`,
it means that presto has already finished the query and superset can stop
polling and get the data back.
After this simple optimization, we get a 2-5x performance boost for
Presto SQL Lab queries.
(cherry picked from commit b71f551)
michellethomas
approved these changes
Jun 18, 2018
Codecov Report
@@ Coverage Diff @@
## airbnb-master #56 +/- ##
=================================================
- Coverage 63.66% 63.65% -0.02%
=================================================
Files 260 260
Lines 19681 19682 +1
Branches 1970 1970
=================================================
- Hits 12530 12528 -2
- Misses 7142 7145 +3
Partials 9 9
Continue to review full report at Codecov.
|
(cherry picked from commit 480ddfc)
michellethomas
pushed a commit
that referenced
this pull request
Dec 1, 2021
* [build] fix typescript builds * [typescript] ensure types pass in build * [typescript][connection] declare modules in tests * [typescript][connection] fix ts errors in tests * [typescript][connection] test/types.ts => types/external.d.ts * [chart][typescript] add @types/react-loadable * [chart][components] convert to ts * [charts][tests][broken] convert to ts * [chart][typescript] re-write component generics * [chart][typescript] fix reactify generic, add react-dom types * [chart][typescript] more iteration * - Tweaking reactify types (using Readonly types). - Uncovered an issue in which ReactifyProps and Props can collide on id and className. - Move @types/react-loadable to dev dependency - Fixing a lint error * [chart][deps] add @types/fetch-mock * [client][typescript] add and export SupersetClientInterface * [chart][clients] fix ts * [charts][components] more ts iterations * [chart][client] assert FormData type * [chart][deps] try adding newest @types/react * [chart][components][ts] fix reactify prop TS * [chart] lint * [chart][ts] lint #2, move @types to deps not dev-deps * [chart][jest] fix tests * [chart][tests] up branch coverage * [chart][ts][test] null => undefined * [chart][tests] hundo * [chart][tests] update name * [chart][ts] ChartClient type fixes
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cherry-picks a few bug fixes and performance changes.
to: @graceguo-supercat @michellethomas @timifasubaa