Add Pagination to Listing Projects for Housekeeping #587

tedkimdev · 2023-07-22T05:28:08Z

What this PR does / why we need it:

To avoid all project info being loaded from DB when doing housekeeping, pagination is added for listing projects and performing housekeeping in smaller portions.

Which issue(s) this PR fixes:

Fixes #569

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Additional documentation:

Checklist:

Added relevant tests or not required
Didn't break anything

CLAassistant · 2023-07-22T05:28:13Z

All committers have signed the CLA.

codecov · 2023-07-22T05:34:10Z

Codecov Report

Merging #587 (5b0705d) into main (036a991) will decrease coverage by 0.22%.
Report is 9 commits behind head on main.
The diff coverage is 67.12%.

@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
- Coverage   51.11%   50.89%   -0.22%     
==========================================
  Files          66       69       +3     
  Lines        7008     7167     +159     
==========================================
+ Hits         3582     3648      +66     
- Misses       2952     3035      +83     
- Partials      474      484      +10

Files Changed	Coverage Δ
server/backend/housekeeping/housekeeping.go	`0.00% <0.00%> (ø)`
server/backend/database/mongo/client.go	`41.96% <71.42%> (+2.54%)`	⬆️
server/backend/database/memory/database.go	`50.28% <84.21%> (-2.86%)`	⬇️
server/backend/housekeeping/config.go	`100.00% <100.00%> (ø)`
server/config.go	`42.52% <100.00%> (-1.38%)`	⬇️

... and 16 files with indirect coverage changes

krapie

Thank you for your contribution!
I left a small request for you.

Since HousekeepingProjectFetchSize option is newly introduced, we need to add this option to the CLI flag.

Ref: #512

server/config.sample.yml

krapie

LGTM 👍🏼

hackerwins

Thanks for your contribution.
I left a comment.

server/backend/database/memory/database.go

hackerwins · 2023-07-31T03:29:11Z

No worries at the moment since we have small projects in https://yorkie.dev, and the offset/limit isn't causing any performance issues.

However, as our services grow, this could become problematic when fetching the first page. To address this potential issue, we could consider changing the interface and implementing it with keyset pagination.

For more information on this approach, you can check out: https://use-the-index-luke.com/no-offset

To avoid all project info being loaded from DB, pagination is added for projects and perform housekeeping in smaller portions. * Add HousekeepingProjectFetchSize in config Fix checking the last page Add housekeeping project fetch size option to the CLI flag * set project fetch size to 100 in config.sample.yml fix lint Move housekeepingProjectPage variable into the Housekeeping.run function Add keyset pagination to find projects * update housekeeping test not to set project id to zero value

tedkimdev · 2023-08-02T04:50:55Z

@hackerwins I changed the database interface like you mentioned.
I did not call SetSort() when using Find() in the Mongodb client because the natural order refers to the default document ordering based on _id.
If I am not correct, please let me know.

krapie

keyset pagination looks awesome!
I left some small comments with my personal questions.

server/config.go

server/backend/housekeeping/housekeeping.go

server/backend/database/memory/database.go

* Cleanup code * add pagination test for memory db Cleanup test

* add test code for mongo client * clean up test code

hackerwins

Thanks for applying my requests.

I left one more comment about using indexes in MemDB.

server/backend/database/memory/database.go

hackerwins

Overall looks good.
I left minor style-related comments.

server/backend/database/database.go

hackerwins · 2023-08-14T22:44:50Z

server/backend/database/memory/housekeeping_test.go

+}
+
+func createDBandProjects(t *testing.T) (*memory.DB, []*database.ProjectInfo) {
+	t.Helper()


This is the first time to see t.Helper. When is this useful? Would it be good to add t.Helper to other helper functions too?

t.Helper() indicates to the Go test runner that createDBandProjects() helper function is a test helper. It makes it easy to report test failures at the call site of the parent function rather than the call site of t.Error() and t.Errorf(). When t.Error() is called from createDBandProjects() function, the Go test runner will report the filename and line number of the code which called the createDBandProjects() function in the output.

server/backend/housekeeping/housekeeping.go

hackerwins

LGTM. 👍

* Add Pagination to Listing Projects for Housekeeping To avoid all project info being loaded from DB, pagination is added for projects and perform housekeeping in smaller portions. * Add HousekeepingProjectFetchSize in config Add housekeeping project fetch size option to the CLI flag * set project fetch size to 100 in config.sample.yml Move housekeepingProjectPage variable into the Housekeeping.run function Add keyset pagination to find projects * update housekeeping test not to set project id to zero value

krapie self-requested a review July 22, 2023 05:38

krapie assigned tedkimdev Jul 22, 2023

krapie added the enhancement 🌟 New feature or request label Jul 22, 2023

krapie requested a review from hackerwins July 22, 2023 05:45

krapie requested changes Jul 22, 2023

View reviewed changes

server/config.sample.yml Outdated Show resolved Hide resolved

tedkimdev requested a review from krapie July 22, 2023 13:01

tedkimdev force-pushed the housekeeping-pagination branch from 834033e to e20e17c Compare July 22, 2023 17:56

krapie approved these changes Jul 25, 2023

View reviewed changes

hackerwins requested changes Jul 26, 2023

View reviewed changes

server/backend/database/memory/database.go Outdated Show resolved Hide resolved

tedkimdev force-pushed the housekeeping-pagination branch from cfd3d0b to 2b58225 Compare August 2, 2023 04:22

tedkimdev force-pushed the housekeeping-pagination branch from 2b58225 to a271e36 Compare August 2, 2023 04:41

tedkimdev requested review from krapie and hackerwins August 2, 2023 04:51

krapie reviewed Aug 4, 2023

View reviewed changes

server/config.go Outdated Show resolved Hide resolved

server/backend/housekeeping/housekeeping.go Show resolved Hide resolved

server/backend/database/memory/database.go Outdated Show resolved Hide resolved

Fix pagination in memory db

9fec852

* Cleanup code * add pagination test for memory db Cleanup test

tedkimdev force-pushed the housekeeping-pagination branch from 5a845cc to 9fec852 Compare August 5, 2023 07:00

Pass last project id as a value

3ff9d60

* add test code for mongo client * clean up test code

tedkimdev force-pushed the housekeeping-pagination branch from 6761e9f to 3ff9d60 Compare August 8, 2023 03:18

hackerwins requested changes Aug 10, 2023

View reviewed changes

server/backend/database/memory/database.go Outdated Show resolved Hide resolved

Use an index in MemDB to list projects

825461b

hackerwins self-requested a review August 11, 2023 00:53

hackerwins reviewed Aug 14, 2023

View reviewed changes

server/backend/housekeeping/housekeeping.go Show resolved Hide resolved

Remove housekeeping prefix from a parameter and add logging error

5b0705d

hackerwins self-requested a review August 15, 2023 01:24

hackerwins approved these changes Aug 15, 2023

View reviewed changes

hackerwins merged commit 21b3afc into yorkie-team:main Aug 15, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Pagination to Listing Projects for Housekeeping #587

Add Pagination to Listing Projects for Housekeeping #587

tedkimdev commented Jul 22, 2023

CLAassistant commented Jul 22, 2023 •

edited

Loading

codecov bot commented Jul 22, 2023 •

edited

Loading

krapie left a comment

krapie left a comment

hackerwins left a comment

hackerwins commented Jul 31, 2023

tedkimdev commented Aug 2, 2023

krapie left a comment

hackerwins left a comment

hackerwins left a comment

hackerwins Aug 14, 2023

tedkimdev Aug 15, 2023

hackerwins left a comment

Add Pagination to Listing Projects for Housekeeping #587

Add Pagination to Listing Projects for Housekeeping #587

Conversation

tedkimdev commented Jul 22, 2023

CLAassistant commented Jul 22, 2023 • edited Loading

codecov bot commented Jul 22, 2023 • edited Loading

Codecov Report

krapie left a comment

Choose a reason for hiding this comment

krapie left a comment

Choose a reason for hiding this comment

hackerwins left a comment

Choose a reason for hiding this comment

hackerwins commented Jul 31, 2023

tedkimdev commented Aug 2, 2023

krapie left a comment

Choose a reason for hiding this comment

hackerwins left a comment

Choose a reason for hiding this comment

hackerwins left a comment

Choose a reason for hiding this comment

hackerwins Aug 14, 2023

Choose a reason for hiding this comment

tedkimdev Aug 15, 2023

Choose a reason for hiding this comment

hackerwins left a comment

Choose a reason for hiding this comment

CLAassistant commented Jul 22, 2023 •

edited

Loading

codecov bot commented Jul 22, 2023 •

edited

Loading