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

Add support for MySQL and other DBs #1139

Closed
wants to merge 2 commits into from
Closed

Conversation

trogper
Copy link
Contributor

@trogper trogper commented Jan 7, 2022

Description

Changes in this PR:
Use Knex to manage database connection and migrations
Connection options are provided via ENV variables.
Rewrite complex (SQLite-only) queries to knex format

Fixes #959 #953

However there is still much work to be done. The current implementation is not backwards compatible with existing databases - there is only one migration, from nothing (blank DB) to current version.

To do

  • convert old db migrations to knex migrations
  • add support for other DBs in knexfile.js
  • wait for knex contributors to fix "table-creator.js" issue
  • decide how to install additional db drivers
  • hide shrink button in settings if DB is not SQLite

Type of change

  • New feature (non-breaking change which adds functionality)
  • Other
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and test it
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • My code needed automated testing. I have added them

convert database migrations to knex,
rewrite complex queries to knex,
Copy link
Contributor

@Saibamen Saibamen left a comment

Choose a reason for hiding this comment

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

Are you sure, that you run ESLint for your code?

knexfile.js Show resolved Hide resolved
knexfile.js Show resolved Hide resolved
server/database.js Outdated Show resolved Hide resolved
server/database.js Show resolved Hide resolved
server/database.js Show resolved Hide resolved
@Saibamen
Copy link
Contributor

Saibamen commented Jan 7, 2022

And IMO it will be better (less bugs, and time needed for code review and tests) to first create PR with Knex (and SQLite) only and adds support for other databases in separate PRs

@trogper
Copy link
Contributor Author

trogper commented Jan 7, 2022

@Saibamen I am sure I have NOT run ESLint 🙂
This PR is very much in the beginning, so I have not bothered. Most of the code submitted here should be rewritten anyways, then I gladly will take care of the formatting.

@Saibamen
Copy link
Contributor

Saibamen commented Jan 7, 2022

+1 for using Knex - this will improve security and standardize SQL queries.

BTW does using Knex will gain some performance improvements for us? (for example, did you notice some faster queries than before)

@trogper
Copy link
Contributor Author

trogper commented Jan 7, 2022

Kuma already uses knex, I have just implemented the migrations and DB connection "management".
No, I don't think using knex gets us any performance improvements. If anything, it adds a bit more overhead to connect and build the queries, but that sould be unnoticeable.

@Saibamen
Copy link
Contributor

Saibamen commented Jan 7, 2022

ahh, right. Thanks

@trogper
Copy link
Contributor Author

trogper commented Jan 7, 2022

My expectations of this PR are suggestions and guidance about how to do things the right way. At the moment I am not going any further with it. Once we settle on how to do thing, I may proceed.

Here are some of my questions:

  • How to test the migrations? Do we have to make an instance of every version of kuma and try to migrate it to knex?
  • When building a docker container, do we include all the drivers (which will unnecessarily bloat it), create multiple containers (kuma-sqlite, kuma-mysql, ...) or somehow install the driver in runtime?
  • Does louslam agree with adding support for another DBs? (I didn't @ him to spam him)
  • How to obtain knex instance in server code? Using R._knex seems rather hacky.

@Saibamen
Copy link
Contributor

Saibamen commented Jan 7, 2022

  • When building a docker container, do we include all the drivers (which will unnecessarily bloat it), create multiple containers (kuma-sqlite, kuma-mysql, ...) or somehow install the driver in runtime?

I suggest separate containers to have smaller image size, because end users will use only one of DB driver

uptime calculation query bugfix
@8ball030
Copy link

8ball030 commented Feb 5, 2022

Multiple containers would work well with helm, as there is another issue in regards to stateful sets failing due to the database.
You could then have a flag in helm as you deployed to specify database configuration required.

@koen20 koen20 mentioned this pull request Mar 13, 2022
1 task
@trogper trogper mentioned this pull request Mar 19, 2022
1 task
@trogper trogper mentioned this pull request Apr 3, 2022
1 task
@premb
Copy link

premb commented Oct 21, 2022

@louislam Thanks a lot for this cool tool.

@trogper I appreciate your efforts to make uptime to support Mysql database. I wanted to migrate sqlite to Mysql, As I'm very new to this, I followed your comments and configuration changes and tried to setup locally. The connection between mysql db and uptime app has been established, but now I encounter UI issues. I found some features are not working as expected when compared sqlite based version. For example: Tags can be defined while adding a new service, but after saving, they are not visible in the panel.

Am i making any mistakes pls correct me.

Screenshot 2022-10-21 at 4 41 45 PM

@trogper
Copy link
Contributor Author

trogper commented Oct 21, 2022

@premb my fork is probably a year old now, there have been many changes since. If you clone my fork, it should work. I will continue making further changes/fixes when louislam agrees to merge it. I don't think it's productive to keep my fork up to date every release.

@louislam do you think this could be merged into 2.0?

@MichalisDBA
Copy link

Hello. Any news or updates about this?

@Maven35
Copy link

Maven35 commented Jan 27, 2023

really interested in having this functionality implemented into uptime-kuma

@flikites
Copy link

Need this functionality. SQlite is too limiting.

@8ball030
Copy link

Upvoted if possible :)

@arnovdk
Copy link

arnovdk commented Mar 11, 2023

Another upvote, from me.

The reason being that I (try to) run Uptime Kuma in a Docker container, which has its volumes on an NFS-share on an external machine (for backup purposes). So being able to use a MySQL-server would be a most welcome feature for me.

Cheers and thanks for this nice piece of software.

@8ball030
Copy link

end the sentiment and appreciation for the fantastic software :)

I have a helm deployment of the application, and i also want to suggest this;

https://pypi.org/project/uptime-kuma-api/

Which can be used to implement backups

@lolen
Copy link

lolen commented Mar 27, 2023

Is there anything known about the progress of work on this feature? Because sqlite is terribly problematic.
@louislam ?

@flikites
Copy link

flikites commented Apr 1, 2023

I don't think @louislam actually gives a hoot about what his users actually want/need.

Kind of a shame, I almost gave this repo a star.

Unfortunately had to find something else because of:

A: No MySQL support

B: Can't create new monitors via an API

@lolen
Copy link

lolen commented Apr 2, 2023

@flikites Probably in the next version there will be support for mysql. But at the moment, using sqlite is very problematic, especially when using a cluster and file system like glusterfs. When a node has a problem, you always have to restore the service from a backup because there is a lack of sqlite consistency. I hope that will change soon.

@flikites
Copy link

flikites commented Apr 2, 2023

@flikites Probably in the next version there will be support for mysql. But at the moment, using sqlite is very problematic, especially when using a cluster and file system like glusterfs. When a node has a problem, you always have to restore the service from a backup because there is a lack of sqlite consistency. I hope that will change soon.

I have a similar use case. Hopefully we will see it in the next release.

@almereyda
Copy link

almereyda commented Apr 2, 2023

Is it known if the SQLite WAL format is supported by Uptime Kuma? This alone could already help to address the resiliency question.

In Postgres land, the WAL allows for Continuous Archiving and Point-in-Time Recovery (PITR). A similar recovery strategy could then apply to Uptime Kuma as well.

Additionally, would there be any field experiences from running this PR's branch in a live environment?

@harryzcy
Copy link
Contributor

harryzcy commented Apr 3, 2023

I came across many issues regarding PostgreSQL/MySQL support, but I still don't know what the current status is. It seems that the work hasn't started despite the community interest.

  • How to test the migrations? Do we have to make an instance of every version of kuma and try to migrate it to knex?

Adding support to other DBs and handling migrations is a huge task. I think it maybe better to start converting hard coded queries to knex way, and keep up-to-date with upstream. This at least ensures original sqlite support is not broken. There's a number of very complex queries, so this task may take a while to get merged.

After all queries are converted, all other DBs should be trivially supported, and we can start handling migration problems. Handling all issues at once may be unrealistic, and is overly complex The issue is frozen for nearly two years and yet no updates, so I don't think it can continue.

  • Does louslam agree with adding support for another DBs? (I didn't @ him to spam him)
  • How to obtain knex instance in server code? Using R._knex seems rather hacky.

I think one issue is if @louislam agree with using knex directly, or we'll have to contribute to redbean-node somehow to make it work. Actually, I see @louislam is using R.knex from redbean-node in v2. Why not merge them into v1 and make changes incremental? I think the community can help with the reviews along the way.

@chakflying
Copy link
Collaborator

Code change is obsolete by #2720. #959 is better tracked in #3748.

@chakflying chakflying closed this Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Postgres support