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.

Sign up for GitHub

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

Jump to bottom

Migrate old logs to new database schema #1828

Merged
merged 29 commits into from
Jun 12, 2023
Merged

Migrate old logs to new database schema #1828

merged 29 commits into from
Jun 12, 2023

Conversation

6543
Copy link
Member

@6543 6543 commented Jun 5, 2023 •
edited
Loading

migration extracted from #1802

address new models based on #1802

@6543 6543 added bug Something isn't working server refactor delete or replace old code regression fix a bug that was not released yet labels Jun 5, 2023
@6543 6543 added this to the 1.0.0 milestone Jun 5, 2023
6543 added a commit to anbraten/woodpecker that referenced this pull request Jun 5, 2023
@6543
move migration into new pull: woodpecker-ci#1828
9eb0273
@6543 6543 mentioned this pull request Jun 5, 2023
Merged
14 tasks
6543 added a commit that referenced this pull request Jun 6, 2023
@anbraten @6543
Rework log streaming and related functions (#1802)
556607b 
closes #1801
closes #1815 
closes #1144
closes  #983
closes  #557
closes #1827
regression of #1791

# TODO
- [x] adjust log model
- [x] add migration for logs
- [x] send log line via grpc using step-id
- [x] save log-line to db
- [x] stream log-lines to UI
- [x] use less structs for log-data
- [x] make web UI work
  - [x] display logs loaded from db
  - [x] display streaming logs
- [ ] ~~make migration work~~ -> dedicated pull (#1828)

# TESTED
- [x] new logs are stored in database
- [x] log retrieval via cli (of new logs) works
- [x] log streaming works (tested via curl & webui)
- [x] log retrieval via web (of new logs) works

---------

Co-authored-by: 6543 <6543@obermui.de>
@6543 6543 marked this pull request as ready for review June 8, 2023 23:11
@6543 6543 requested a review from a team June 8, 2023 23:11
@6543
Copy link
Member Author

6543 commented Jun 9, 2023 •
edited
Loading

sqlite manual migration test (perPage=500):

  • db size: 12.1 GiB
  • select count(log_id) from logs;: 269737
  • processed pages: 540
  • duration HDD: 155min
  • average HDD write: ~30Mb/s
  • peak RAM consumption during migration: ~25Gib
  • db size afterwards: 23G
  • compression: OFF
  • vacuum sized: 11GiB

@6543
Copy link
Member Author

6543 commented Jun 9, 2023 •
edited
Loading

sqlite manual migration test (perPage=5000):

  • db size: 12.1 GiB
  • select count(log_id) from logs;: 269737
  • processed pages: 54
  • duration SSD: 128min
  • average SSD write: ~120Mb/s
  • peak RAM consumption during migration: ~26Gib
  • db size afterwards: 23G
  • compression: OFF
  • vacuum sized: 11GiB

@6543
Copy link
Member Author

6543 commented Jun 9, 2023 •
edited
Loading

sqlite manual migration test (perPage=500):

  • db size: 12.1 GiB
  • select count(log_id) from logs;: 269737
  • processed pages: 540
  • duration SSD: 101min
  • average SSD write: ~50Mb/s
  • peak RAM consumption during migration: ~22Gib
  • db size afterwards: 21G
  • compression: ON ( WIP: Compress LogEntry Data  #1844 zstd)
  • vacuum sized: 8.7GiB

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2023 •
edited
Loading

Codecov Report

Patch coverage: 50.90% and project coverage change: +0.16 🎉

Comparison is base (b5b3b95) 39.18% compared to head (582c1db) 39.34%.

❗ Current head 582c1db differs from pull request most recent head f9acacc. Consider uploading reports for the commit f9acacc to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1828      +/-   ##
==========================================
+ Coverage   39.18%   39.34%   +0.16%     
==========================================
  Files         172      173       +1     
  Lines       10525    10615      +90     
==========================================
+ Hits         4124     4177      +53     
- Misses       6134     6157      +23     
- Partials      267      281      +14
Impacted Files Coverage Δ
server/store/datastore/migration/migration.go 46.22% <37.03%> (+3.80%) ⬆️
.../store/datastore/migration/019_alter_logs_table.go 55.42% <55.42%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@6543 6543 added the wip label Jun 11, 2023
@6543
Copy link
Member Author

6543 commented Jun 11, 2023

ok did make so that ram requirement stays at 60Mb

@6543
Copy link
Member Author

6543 commented Jun 11, 2023

image

@6543 6543 removed the wip label Jun 11, 2023
@woodpecker-bot
Copy link
Contributor

woodpecker-bot commented Jun 11, 2023 •
edited
Loading

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-1828.surge.sh

@6543
Copy link
Member Author

6543 commented Jun 12, 2023

mariadb manual migration test (perPage=100):

  • select count(log_id) from logs;: 269737
  • processed pages: 2698
  • duration SSD: ~4h
  • peak RAM consumption during migration: 323Mb

@6543
Copy link
Member Author

6543 commented Jun 12, 2023

mariadb manual migration test (perPage=100):

  • select count(log_id) from logs;: 500
  • processed pages: 5
  • duration SSD: 3s
  • peak RAM consumption during migration: 66Mb

@6543 6543 requested a review from qwerty287 June 12, 2023 17:42
qwerty287
qwerty287 approved these changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code, this seems fine to me. I didn't test it yet though. If you can wait for the next days, I can do this, but I'm also fine with not testing as it looks like you did that already pretty much.

@6543 6543 enabled auto-merge (squash) June 12, 2023 20:02
@6543 6543 merged commit 5139624 into woodpecker-ci:master Jun 12, 2023
@6543 6543 deleted the fix-log_migration branch June 12, 2023 21:21
@6543 6543 mentioned this pull request Jun 12, 2023
Merged
lafriks pushed a commit that referenced this pull request Jun 13, 2023
@6543
logs table migration: skip on corrupted json and fail later (#1856)
6be61e6 
enhance  #1828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qwerty287 qwerty287 qwerty287 approved these changes

Assignees
No one assigned
Labels
bug Something isn't working refactor delete or replace old code regression fix a bug that was not released yet server
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@6543 @codecov-commenter @woodpecker-bot @qwerty287