[AMORO-2893] optimize table page loading

[klion26]

Why are the changes needed?

Close #2893 .

Optimize the loading process when user open the optimizer page

Brief change log

How was this patch tested?

• [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

• [ ] Add screenshots for manual tests if appropriate

• [ ] Run test locally before making a pull request

Documentation

• Does this pull request introduce a new feature? (yes / no)
• If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

[klion26]

Currently, this is a draft version just a better discussion. the open questions are

1. Do we need to include the table format in what we return to the front end, which right now doesn't seem to be using it?
2. If we don't need to need to include the table format, which way do we prefer to add the infor no in table_runtime 2.1. select info from DB for the given tables retrieved in the previous step 2.2. retrieve the info from TableManager(for example add a function TableManager#getTableRuntime(int tableId)), which needs to maintain an tableId to tableRuntime relationship in memory

I'm leaning towards option 2.2 with not returning the table format to the front end because it reduces the number of DB requests by one

Please let me what you think about this, thanks. @zhoujinsong @majin1102

[baiyangtx]

Now that you have queried the TableRuntimeBean object from the database, why not directly perform sorting or paging operations on this object later?

[majin1102]

Currently, this is a draft version just a better discussion. the open questions are

1. Do we need to include the table format in what we return to the front end, which right now doesn't seem to be using it?
2. If we don't need to need to include the table format, which way do we prefer to add the infor no in table_runtime 2.1. select info from DB for the given tables retrieved in the previous step 2.2. retrieve the info from TableManager(for example add a function TableManager#getTableRuntime(int tableId)), which needs to maintain an tableId to tableRuntime relationship in memory

I'm leaning towards option 2.2 with not returning the table format to the front end because it reduces the number of DB requests by one

Please let me what you think about this, thanks. @zhoujinsong @majin1102

I think decoupling from TableService and TableManager is a good idea. We may seperate dashboard server from ams in the future. TableFormat seems unnecessary in this page for now

[klion26]

@baiyangtx thanks for the review, the bottle-neck here is not the cost of the sort, but we retrieved too much data from DB, the solution here wants to limit the row number when retrieved from DB(the sort in SQL is needed because we want to show the 'running' status before the 'idle' status in the frontend)

[klion26]

@majin1102 sorry for the late reply, I've drafted a version such that: TableRuntimeMeta is used for retrieving from db, TableRuntime is used for data transfer object in ams. could you please take a took if this is the right way when you're free? thanks.

[klion26]

@majin1102 @zhoujinsong @baiyangtx I've updated a version, could you please take a look at this when you're free, thanks

[klion26]

@baiyangtx thanks very much for the review, will update the PR % the search pattern about the db soon, that change will change after we reach the consensus.

[klion26]

@zhoujinsong thanks very much for the review, will update it and attach some result before and after apply the change

[github-actions[bot]]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

[klion26]

sorry for the late reply, will rebase the latest master and attach the result asap.

[klion26]

Finally, got the compare result before and after applying the change.

Short answer: This modification has a particularly good performance improvement。 Before the change, the page can be loaded in 45s(this is the default timeout), after the change, the page loading will use some seconds such as below

To quickly obtain comparison results in the production environment, no relevant indexes are added to the database. That is to say, after the current PR is merged, the loading effect should be better (because we added indexes to the fields).

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 22.42%. Comparing base (8207efc) to head (bb4475b). Report is 27 commits behind head on master.

:exclamation: There is a different number of reports uploaded between BASE (8207efc) and HEAD (bb4475b). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (8207efc)	HEAD (bb4475b)
core	2	0

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2914      +/-   ##
============================================
- Coverage     29.43%   22.42%   -7.01%     
+ Complexity     3685     2280    -1405     
============================================
  Files           568      379     -189     
  Lines         47267    38044    -9223     
  Branches       6196     5439     -757     
============================================
- Hits          13912     8532    -5380     
+ Misses        32372    28779    -3593     
+ Partials        983      733     -250     

Flag	Coverage Δ
core	?
trino	22.42% <ø> (?)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[majin1102]

Sorry for being late. look great to me

[klion26]

@zhoujinsong @majin1102 thanks for the review and merging!