You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/01/31 02:42:29 UTC

[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled

graceguo-supercat opened a new pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060
 
 
   ### CATEGORY
   
   Choose one
   
   - [x] Bug Fix
   - [x] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   When user opens a large dashboard, it may trigger a large amount of data request to backend. Most browser only allow 6 concurrent requests to the same domain. So we use sub-domains (and domain sharding) in airbnb to allow more concurrent requests. If the chart data is well cached, the result of domain-sharding is very positive.
   
   When user open a large dashboard and its content is not well cached, all the slots are blocked by slow queries. At the same time if user tries to make API call to backend, there will be no response in the page. for example, check/uncheck fav star, save dashboard edit etc. This make user confused and feel dashboard is broken.
   
   If we enable domain sharding, we can allocate a domain for user events. This PR is to free the main domain away from chart loading, so that user interactivity won't be blocked by slow queries.
   
   ### TEST PLAN
   added unit test
   
   
   
   ### REVIEWERS
   @etr2460 @robdiciuccio @mistercrunch 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060#discussion_r373678985
 
 

 ##########
 File path: superset/assets/src/explore/exploreUtils.js
 ##########
 @@ -32,6 +32,12 @@ let requestCounter = 0;
 function getHostName(allowDomainSharding = false) {
   let currentIndex = 0;
   if (allowDomainSharding) {
+    // if domain sharding is enabled, skip main domain for fetching chart API
+    // leave main domain free for other calls like fav star, save change, etc.
+    // to make dashboard be responsive when it's loading large number of charts
+    if (requestCounter % availableDomains.length === 0) {
+      requestCounter += 1;
+    }
 
 Review comment:
   changes are added.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] etr2460 commented on a change in pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled

Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060#discussion_r373586638
 
 

 ##########
 File path: superset/assets/src/explore/exploreUtils.js
 ##########
 @@ -32,6 +32,12 @@ let requestCounter = 0;
 function getHostName(allowDomainSharding = false) {
   let currentIndex = 0;
   if (allowDomainSharding) {
+    // if domain sharding is enabled, skip main domain for fetching chart API
+    // leave main domain free for other calls like fav star, save change, etc.
+    // to make dashboard be responsive when it's loading large number of charts
+    if (requestCounter % availableDomains.length === 0) {
+      requestCounter += 1;
+    }
 
 Review comment:
   minor nit: since you're duplicating the `requestCounter % availableDomains.length` logic from below, might it be simpler to do:
   
   ```js
   if (allowDomainSharding) {
     currentIndex = requestCounter % availableDomains.length;
     requestCounter += 1;
   
     // if domain sharding is enabled, skip main domain for fetching chart API
     // leave main domain free for other calls like fav star, save change, etc.
     // to make dashboard be responsive when it's loading large number of charts
     if (currentIndex === 0) {
       currentIndex += 1;
       requestCounter += 1;
     }
   }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io commented on issue #9060: [domain sharding] Freeup main domain when domain sharding is enabled

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060#issuecomment-580559716
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=h1) Report
   > Merging [#9060](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2629c779af10b18d669d9af8612b92b7138bdd38?src=pr&el=desc) will **increase** coverage by `<.01%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/9060/graphs/tree.svg?width=650&token=KsB0fHcx6l&height=150&src=pr)](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9060      +/-   ##
   ==========================================
   + Coverage   59.45%   59.46%   +<.01%     
   ==========================================
     Files         369      369              
     Lines       11747    11749       +2     
     Branches     2888     2889       +1     
   ==========================================
   + Hits         6984     6986       +2     
     Misses       4584     4584              
     Partials      179      179
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/assets/src/explore/exploreUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/9060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2V4cGxvcmVVdGlscy5qcw==) | `76.63% <100%> (+0.44%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=footer). Last update [2629c77...5bdfedf](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] graceguo-supercat merged pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled

Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on issue #9060: [domain sharding] Freeup main domain when domain sharding is enabled

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9060: [domain sharding] Freeup main domain when domain sharding is enabled
URL: https://github.com/apache/incubator-superset/pull/9060#issuecomment-580559716
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=h1) Report
   > Merging [#9060](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2629c779af10b18d669d9af8612b92b7138bdd38?src=pr&el=desc) will **increase** coverage by `0.01%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/9060/graphs/tree.svg?width=650&token=KsB0fHcx6l&height=150&src=pr)](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9060      +/-   ##
   ==========================================
   + Coverage   59.45%   59.46%   +0.01%     
   ==========================================
     Files         369      369              
     Lines       11747    11750       +3     
     Branches     2888     2889       +1     
   ==========================================
   + Hits         6984     6987       +3     
     Misses       4584     4584              
     Partials      179      179
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <100%> (ø)` | :arrow_up: |
   | [superset/assets/src/explore/exploreUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/9060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2V4cGxvcmVVdGlscy5qcw==) | `76.85% <100%> (+0.66%)` | :arrow_up: |
   | [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `54.21% <0%> (ø)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=footer). Last update [2629c77...5b6bfcf](https://codecov.io/gh/apache/incubator-superset/pull/9060?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org