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

Switch to diesel #239

Merged
merged 21 commits into from
Nov 21, 2024
Merged

Switch to diesel #239

merged 21 commits into from
Nov 21, 2024

Conversation

jr1221
Copy link
Contributor

@jr1221 jr1221 commented Nov 10, 2024 •
edited by bracyw
Loading

Switches to diesel.

  • embed migrations
  • fix service functions
  • figure out purpose of transformers
  • make database URL use dotenv
  • edit db_handler location
  • fix tests
  • fix dockerfile, docker compose
  • Add back run upserting with location support
  • Remove dashes to slashes

Add support for setting note (SAVE FOR STARTER SCYLLA TICKET)

@RChandler234 RChandler234 mentioned this pull request Nov 10, 2024
Closed
11 tasks
@jr1221
Copy link
Contributor Author

jr1221 commented Nov 14, 2024

Current wierd stuff.

  • Max batch upsert limit of 65535 means we may have to implement our own chunking and saturation logic
  • Diesel is not obeying null constraints for the inserts????

Current breaking changes

  • No more mock
  • No more seed
  • No more prod mode setting or saturate batches setting

Current improvements noted:

  • No more cloning stuff in transformer layer due to into_iter over iter
  • Embedded migrations means faster, smaller builds
  • Much less dependencies to build

@jr1221
Copy link
Contributor Author

jr1221 commented Nov 14, 2024

See above TODO for what is left.
Current concerns for regressions:

  • DB batch uploader must split to chunks than copy back into vec
  • DB batch uploader must open so many parallel connections

@jr1221
Copy link
Contributor Author

jr1221 commented Nov 14, 2024 •
edited
Loading

Image size before: 3.36gb
Image size after: 88mb 😎

Build time on GHA before: 1hr 40min
Build time on GHA after: 36min

@jr1221
Copy link
Contributor Author

jr1221 commented Nov 20, 2024 •
edited
Loading

Closes #175
Closes #231

This was linked to issues Nov 20, 2024
Change - separator to / separator to match Calypso #175
Closed
[Scylla] - Optimize docker build #231
Closed
@bracyw bracyw marked this pull request as ready for review November 20, 2024 22:18
RChandler234
RChandler234 previously requested changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@RChandler234 RChandler234 left a comment

Choose a reason for hiding this comment

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

Beautiful
image

compose/compose.client-dev.yml Show resolved Hide resolved
compose/compose.yml Outdated Show resolved Hide resolved
scylla-server/README.md Outdated Show resolved Hide resolved
scylla-server/src/db_handler.rs Show resolved Hide resolved
scylla-server/src/mqtt_processor.rs Show resolved Hide resolved
scylla-server/src/services/data_service.rs Show resolved Hide resolved
scylla-server/src/transformers/run_transformer.rs Outdated Show resolved Hide resolved
@bracyw bracyw removed the request for review from RChandler234 November 21, 2024 07:28
@bracyw bracyw dismissed RChandler234’s stale review November 21, 2024 07:28

Changes addressed

@bracyw bracyw merged commit f7ca9b4 into develop Nov 21, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RChandler234 RChandler234 RChandler234 left review comments

@bracyw bracyw bracyw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Scylla] - Optimize docker build Change - separator to / separator to match Calypso
3 participants
@jr1221 @RChandler234 @bracyw