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

Use TiDB instead of MySQL as default database #60

Open
lastzero opened this Issue Nov 9, 2018 · 25 comments

Comments

6 participants
@lastzero
Copy link
Member

lastzero commented Nov 9, 2018

As a potential user, I don't want to run a full blown MySQL server in order to simplify installation and reduce memory usage.

We solved this by replacing MySQL with TiDB, a compatible database server implemented in pure Go. Most of the changes are done! Our demo already runs on TiDB.

It starts automatically, if tidb is selected as database-driver. Initialization and configuration should be improved, still a bit experimental at the moment. Also we need test coverage and some code clean-up.

Acceptance Criteria:

  • TiDB MUST be initialized with photoprism as user, password and database without restart; using FLUSH PRIVILEGES should work (see comment)
  • TiDB root password MUST be photoprism (can be changed by user during setup later, not part of this ticket)
  • Integration tests SHOULD run on TiDB as well (MySQL still used right now, see docker-compose.yml); started working on photoprism/tidb Docker image for this
  • Code clean-up in /internal/tidb SHOULD be performed
  • Unit tests MUST be added
  • We should ask the TiDB developers if they have any concerns.

@lastzero lastzero added this to the MVP milestone Nov 9, 2018

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Nov 16, 2018

We are now definitively working on a single binary for distribution. Many users certainly don't want to install Docker (and keep it updated).

@LeKovr LeKovr referenced this issue Nov 20, 2018

Closed

Postgres support? #47

@lastzero lastzero added go and removed go labels Nov 24, 2018

@robmarkcole

This comment has been minimized.

Copy link

robmarkcole commented Dec 2, 2018

@lastzero how can I help? I am a python dev but very keen on supporting this project

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 4, 2018

@robmarkcole Thank you for reaching out! A good start is to join our developers mailing list: https://groups.google.com/a/photoprism.org/forum/#!forum/developers

This is a young project. We are in the process of organizing work and building a community. We have no issues that require Python at the moment (possibly later). Maybe you're interested in doing research (like testing different RAW to JPEG converters) or learning Go?

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 5, 2018

From what I learned, it requires significant work to use sqlite3 concurrently. Before we start refactoring, let's see if we can run a MySQL- or PostgreSQL-compatible server within PhotoPrism:

  • TiDB - a distributed HTAP database compatible with the MySQL protocol
  • CockroachDB - ultra-resilient SQL for global business (PostgreSQL protocol)

It seems like a crazy idea, but why not?

@robmarkcole

This comment has been minimized.

Copy link

robmarkcole commented Dec 5, 2018

HI @lastzero I am interested in learning Go, and know some SQL. In my day job I work with satellite imagery, and several services I use have PostgreSQL as a back-end. If theres a task in this area that will get me using Go please let me know :)

@webrules

This comment has been minimized.

Copy link
Contributor

webrules commented Dec 7, 2018

I had a look at TiDB. It can be an (almost) drop-in replacement for MySQL. However, only Docker or manual installations on Linux are available. Guess it may not help on the vision of single binary distribution of PhotoPrism?

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 7, 2018

@webrules It seems like you can build TiDB on Windows: pingcap/tidb#4295

I would actually not build a separate binary but compile this into our own. Should be possible, as long as there are no external dependencies, like shared libraries that are only available on Linux.

@webrules

This comment has been minimized.

Copy link
Contributor

webrules commented Dec 7, 2018

A bit confused. Does that mean you want to replace MySQL in the docker image with TiDB or CockroachDB? Or it is more than that?

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 7, 2018

We don't want to use Docker anymore. Essentially, PhotoPrism would be its own database server.

@webrules

This comment has been minimized.

Copy link
Contributor

webrules commented Dec 7, 2018

Wow, that'll be a big change. Thank you!

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 10, 2018

File Browser seems to use SQLite3 - we should check their implementation.

@jed-frey

This comment has been minimized.

Copy link

jed-frey commented Dec 13, 2018

I would suggest using an ORM so that the backend can be configurable.

http://xorm.io/

@lastzero lastzero self-assigned this Dec 18, 2018

lastzero added a commit that referenced this issue Dec 18, 2018

@IssuehuntBot

This comment has been minimized.

Copy link

IssuehuntBot commented Dec 19, 2018

@IssueHuntFest has funded $60.00 to this issue. See it on IssueHunt

@lastzero lastzero changed the title Use sqlite3 instead of mysql as default database Use TiDB instead of MySQL as default database Dec 19, 2018

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 19, 2018

Most of the changes are basically done! Our demo already runs on TiDB.

Added remaining tasks to issue description.

@c4pt0r

This comment has been minimized.

Copy link

c4pt0r commented Dec 19, 2018

Amazing work! A member from TiDB here, our single node mode is only used for running unit tests. If you want to put it in production, I think cluster mode is a must.

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 19, 2018

@c4pt0r Thank you! What can happen in the worst case? This software will mainly run on desktop computers and other small devices, so we can't use a cluster by default. We will keep metadata in XMP sidecar files, so a database crash is not the end of the world.

@jed-frey

This comment has been minimized.

Copy link

jed-frey commented Dec 19, 2018

This software will mainly run on desktop computers and other small devices.

Has that design decision been made? While I am an atypical user, I've been shooting digital photos since 2002. I have ~1.5TB of pictures all archived on my NAS. I'm slowly hacking through my own solution in python, but I can run locally but scale to every core in my house to scan EXIF and images.

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 19, 2018

@jed-frey A NAS is more similar to a desktop computer than to typical cloud computing environments with lots of nodes far away from home. There is a tradeoff between scalability and complexity. Thank god, modern computers - even smartphones - are very powerful and can easily handle 1.5 TB, given enough storage capacity.

@c4pt0r

This comment has been minimized.

Copy link

c4pt0r commented Dec 19, 2018

@c4pt0r Thank you! What can happen in the worst case? This software will mainly run on desktop computers and other small devices, so we can't use a cluster by default. We will keep metadata in XMP sidecar files, so a database crash is not the end of the world.

Cool, I got your point, I think it's not a big deal. TiDB uses goleveldb as the underlying storage for single node mode, and it's solid enough for desktop application. And I think it's pretty cool if you can put tidb and photoprism into one binary.

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 19, 2018

@c4pt0r It would be amazing if we could initialize TiDB with a user, password and database without restart. Either we add this functionality to TiDB somehow (what is the best way?), or we ship our app with existing database files. Would they be binary compatible for all platforms (processors and operating systems)?

@c4pt0r

This comment has been minimized.

Copy link

c4pt0r commented Dec 19, 2018

@c4pt0r It would be amazing if we could initialize TiDB with a user, password and database without restart. Either we add this functionality to TiDB somehow (what is the best way?), or we ship our app with existing database files. Would they be binary compatible for all platforms (processors and operating systems)?

I think shipping with existing database file is not a good idea, I suggest to putting create table statements with your application, execute them when bootstrapping. It's a more flexible way.

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 19, 2018

@c4pt0r Why did you change your mind? The problem with create statements is, that the client doesn't know if they've already been executed plus the server could be attacked as root without password for at least a short period of time. This is a quick hack to create a database:

https://github.com/photoprism/photoprism/blob/develop/internal/tidb/init.go

Changing the root password or other user credentials however only seems to take effect after server restart, that's why I removed it again. Bug or feature? Thank you very much for your advice 👍

@c4pt0r

This comment has been minimized.

Copy link

c4pt0r commented Dec 20, 2018

You need to run 'FLUSH PRIVILEGES;' to make it work, instead of restarting the server.

MYSQL> SET PASSEWORD='new_password';
MYSQL> FLUSH PRIVILEGES;

the doc is here: https://github.com/pingcap/docs/blob/master/sql/privilege.md#time-of-effect

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Dec 20, 2018

Ah, great. Thought it is not implemented because of pingcap/tidb#674. Top position when you google for "FLUSH PRIVILEGES tidb".

https://github.com/pingcap/docs/blob/master/sql/user-account-management.md also doesn't mention it. I'll send a PR.

@lastzero

This comment has been minimized.

Copy link
Member

lastzero commented Jan 6, 2019

Maybe it's possible for us to directly run SQL statements like this:

import (
        "github.com/pingcap/tidb/sessionctx"
	"github.com/pingcap/tidb/util"
	"github.com/pingcap/tidb/util/sqlexec"
)

func addUser(ctx sessionctx.Context) {
	_, _, err := ctx.(sqlexec.RestrictedSQLExecutor)
           .ExecRestrictedSQL(ctx, 
               `CREATE USER 'photoprism'@'localhost' IDENTIFIED BY 'photoprism'`
           )
	if err != nil {
		log.Errorf("unable to create user: %s", err)
	}
}

See tidb/domain/domain.go for more examples.

CaitinChen added a commit to pingcap/docs that referenced this issue Jan 10, 2019

sql: add FLUSH PRIVILEGES hint (#835)
* Add FLUSH PRIVILEGES hint to user-account-management.md and FAQ.md

See photoprism/photoprism#60 (comment)

* FLUSH PRIVILEGES only required if grant table was modified

* Fix wording: grant tables

* Change wording as requested

* Change link as requested

* Use backticks to format title as code

Co-Authored-By: lastzero <michael@lastzero.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment