Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

render the tables list even as the table sizes are loading #9741

Merged
merged 4 commits into from
Nov 8, 2022
Merged

render the tables list even as the table sizes are loading #9741

merged 4 commits into from
Nov 8, 2022

Conversation

jadami10
Copy link
Contributor

@jadami10 jadami10 commented Nov 6, 2022

Reopening from #9198 because I couldn't figure out the mess I made between rebase and merging.

This is a ui feature

This will fix something that's annoyed me greatly about the tables page. We fetch the table list, then the table data, then the schema data. But we don't render the tables until we have all that data. Since this can take minutes (specifically getting the size of big tables), it leaves the page on a spinner when usually we just want the table list immediately.

  • modifies the TableData model to track if the data is still loading
  • modifies Tables.tsx to render a loading indicator
  • modifies TablesListingPage.tsx to separately load schema and table data
  • modifies Tables.tsx to track initial data separately from the data that is finally rendered so we can update the table data while still maintaining sorts and filters.
async_table.mov

@jadami10
Copy link
Contributor Author

jadami10 commented Nov 6, 2022

@Jackie-Jiang, took me quite a while to get back to to this. But pulling your old comment here

This is a great feature!

Let's list down all the data need to be fetched, and we may decide which ones should be loaded sync and which ones async.
Also, I'm not sure if we want to query the table size on the top level page. In an environment with hundreds or even more tables, that will create quite some traffic to the servers. We should limit the details to the table level page when there are too many tables (if not already done that way).

If we order the cost of operations from least expensive to most expensive:

List tables / schemas
Read schema to get column stats
Read table IS and EV to get segment availability
Read table size from ser

I think this PR I wanted to limit this to just the async change and not actually change any of the data loaded here.

@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2022

Codecov Report

Merging #9741 (72f7eb6) into master (aa013a4) will increase coverage by 32.05%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master    #9741       +/-   ##
=============================================
+ Coverage     28.23%   60.28%   +32.05%     
- Complexity       53     5234     +5181     
=============================================
  Files          1939     1939               
  Lines        104208   104208               
  Branches      15798    15798               
=============================================
+ Hits          29426    62825    +33399     
+ Misses        71914    36702    -35212     
- Partials       2868     4681     +1813     
Flag Coverage Δ
integration1 ?
integration2 24.42% <ø> (-0.06%) ⬇️
unittests1 67.56% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...g/apache/pinot/server/api/resources/ErrorInfo.java 0.00% <0.00%> (-100.00%) ⬇️
...pinot/minion/exception/TaskCancelledException.java 0.00% <0.00%> (-100.00%) ⬇️
...ot/broker/requesthandler/BrokerRequestHandler.java 0.00% <0.00%> (-100.00%) ⬇️
...ker/failuredetector/ConnectionFailureDetector.java 0.00% <0.00%> (-100.00%) ⬇️
...minion/tasks/mergerollup/MergeRollupTaskUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...ion/tasks/mergerollup/MergeRollupTaskExecutor.java 0.00% <0.00%> (-100.00%) ⬇️
...nverttorawindex/ConvertToRawIndexTaskExecutor.java 0.00% <0.00%> (-100.00%) ⬇️
...e/pinot/common/minion/MergeRollupTaskMetadata.java 0.00% <0.00%> (-94.74%) ⬇️
...plugin/segmentuploader/SegmentUploaderDefault.java 0.00% <0.00%> (-87.10%) ⬇️
...or/BaseExponentialBackoffRetryFailureDetector.java 0.00% <0.00%> (-85.72%) ⬇️
... and 1207 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Jackie-Jiang Jackie-Jiang added feature ui UI related issue labels Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ui UI related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants