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

Postgres database does not created if it's not named as tinode #860

Closed
2 tasks done
Makashov opened this issue May 2, 2023 · 12 comments
Closed
2 tasks done

Postgres database does not created if it's not named as tinode #860

Makashov opened this issue May 2, 2023 · 12 comments
Labels

Comments

@Makashov
Copy link
Contributor

Makashov commented May 2, 2023

PostgreSQL migration

When I compile and run tinode-db using PostgreSQL and set the database name in tinode.conf to something other than 'tinode', the database is not created and program panics.

Your environment

Server-side

  • Your own setup:
    • platform (Mac)
    • version of Tinode server, v0.22.7`
    • database backend postgres adapter version 113
    • standalone

Client-side

  • TinodeWeb/tinodejs: javascript client
    • Browser make and version.
    • IMPORTANT! Use index-dev.html to reproduce the problem, not index.html.

Steps to reproduce

Rename database name in tinode-db/tinode.conf to something other than tinode.
Run tinode-db binary file.

Expected behaviour

Create database specified in tinode-db/tinode.conf if it doesn't exists. And all required tables.

Actual behaviour

Program panics without any explanation of the error. Just a debug information from panic.

Server-side log

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x133e05d]

goroutine 1 [running]:
github.com/jackc/pgx/v4/pgxpool.(*Pool).Acquire(0x0, {0x147fd40, 0xc0001a4008})
        /Users/nurbol/go/pkg/mod/github.com/jackc/pgx/v4@v4.18.1/pgxpool/pool.go:532 +0x3d
github.com/jackc/pgx/v4/pgxpool.(*Pool).QueryRow(0x0?, {0x147fd40, 0xc0001a4008}, {0x13f0a78, 0x27}, {0xc000191a30, 0x1, 0x1})
        /Users/nurbol/go/pkg/mod/github.com/jackc/pgx/v4@v4.18.1/pgxpool/pool.go:641 +0x55
github.com/tinode/chat/server/db/postgres.(*adapter).GetDbVersion(0xc0001a2660)
        /Users/nurbol/Documents/go/tinode-chat/server/db/postgres/adapter.go:217 +0xf7
github.com/tinode/chat/server/store.storeObj.GetDbVersion(...)
        /Users/nurbol/Documents/go/tinode-chat/server/store/store.go:170
main.main()
        /Users/nurbol/Documents/go/tinode-chat/tinode-db/main.go:245 +0x999

Client-side log

Have no logs

@Makashov Makashov added the bug label May 2, 2023
@Makashov
Copy link
Contributor Author

Makashov commented May 2, 2023

I found that in server/db/postgres/adapter.go there is an if/else statement where is the database name is hardcoded I guess.

// Ignore missing database here. If we are initializing the database
// missing DB is OK.
if a.poolConfig.ConnConfig.Database == "tinode" {
	a.poolConfig.ConnConfig.Database = "postgres"
	a.db, err = pgxpool.ConnectConfig(ctx, a.poolConfig)
}

I tried to change the tinode string to the actual name of my database and it worked as I expected. But since I am very new in this project and in golang development I am not sure for what this statement for.
I also can't understand what a.poolConfig.ConnConfig.Database = "postgres" does.

P.S. Thank you for your great work!

@or-else
Copy link
Contributor

or-else commented May 2, 2023

Thanks, fixed in 52f33fe

@or-else
Copy link
Contributor

or-else commented May 4, 2023

Have you had a chance to verify?

@Makashov
Copy link
Contributor Author

Makashov commented May 5, 2023

For some reason it tries to create database named tinode and ignores the configs. As I understood it gets it from defaultDatabase constant.

@Makashov
Copy link
Contributor Author

Makashov commented May 5, 2023

Ok I got it. It happens only when postgres.dsn is used in configs. But if you use the DBName everything is fine.

@or-else
Copy link
Contributor

or-else commented May 6, 2023

Can you share your config? I cannot reproduce.

@or-else
Copy link
Contributor

or-else commented May 6, 2023

I removed some unnecessary logic: b7817c7
Please see if the problem persists.

@Makashov
Copy link
Contributor Author

Makashov commented May 6, 2023

My config is below. Notice I specified database name as my_db but it created database named tinode:

{
	"store_config": {
		"uid_key": "la6YsO+bNX/+XIkOqc5Svw==",
		"use_adapter": "",
		"adapters": {
			"postgres": {
				"dsn": "postgresql://postgres:postgres@localhost:5432/my_db?sslmode=disable"
			},
			"mysql": {
				"database": "tinode",
				"dsn": "root@tcp(localhost)/tinode?parseTime=true&collation=utf8mb4_unicode_ci"
			},
			"rethinkdb": {
				"database": "tinode",
				"addresses": "localhost:28015"
			},
			"mongodb": {
				"database": "tinode",
				"addresses": "localhost:27017"
			}
		}
	}
}

@or-else
Copy link
Contributor

or-else commented May 6, 2023

That's expected. For historical reasons in addition to DSN, the config also needs to specify the database name separately as dbname. I'll see how hard it would be to fix it.

@Makashov
Copy link
Contributor Author

Makashov commented May 6, 2023

Yes worked after adding "DBName": "my_db"

@or-else
Copy link
Contributor

or-else commented May 7, 2023

I removed the requirement for specifying db name in addition to DSN: 8491f05
DSN alone is now enough.

@or-else
Copy link
Contributor

or-else commented May 8, 2023

I assume everything is fixed here and working as expected. If not, please let me knwo.

@or-else or-else closed this as completed May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants