broken 'edgedb project init'

1st1 commented 2 years ago

Suppose a project has migrations. A user runs edgedb project init and answers "n" to “Do you want to start instance automatically on login? [y/n]”.

edgedb project init then DOESN'T start the instance and fails with applying migrations.

~/d/t/edgedb (main *) » edgedb project init
Found `edgedb.toml` in /Users/yury/dev/tmp/edgedb
Initializing project...
Specify the name of EdgeDB instance to use with this project [default: edgedb]:
> edgedb
Do you want to start instance automatically on login? [y/n]
>
edgedb error: Please answer Y or N
> n
Checking EdgeDB versions...
┌────────────────────────┬────────────────────────────────────────┐
│ Project directory      │ /Users/yury/dev/tmp/edgedb             │
│ Project config         │ /Users/yury/dev/tmp/edgedb/edgedb.toml │
│ Schema dir (non-empty) │ /Users/yury/dev/tmp/edgedb/dbschema    │
│ Installation method    │ portable package                       │
│ Start configuration    │ manual                                 │
│ Version                │ 1.3+804c096                            │
│ Instance name          │ edgedb                                 │
└────────────────────────┴────────────────────────────────────────┘
Downloading package...
00:00:00 [====================] 32.58MiB/32.58MiB 34.06MiB/s | ETA: 0s
Successfully installed 1.3+804c096
Initializing EdgeDB instance...
Applying migrations...
Applied m1gcnribdubi476w7daqzhuxvbutfpiqyhhbm2nyfv3xhzc6z757ia (00001.edgeql)
Note: adding first migration disables DDL. More info: https://edgedb.com/p/bare_ddl
[edgedb] WARNING 96328 2022-04-16T14:38:17.436 edb.server: Released an unhealthy pgcon; discard now.
[edgedb] ---- Exception occurred ----
[edgedb]
[edgedb] 1. ConnectionAbortedError:
[edgedb]
[edgedb] ---- Traceback ----
[edgedb]
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 1156, in task
[edgedb]         > await self.introspect_db(dbname)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 679, in introspect_db
[edgedb]         > user_schema = await self.introspect_user_schema(conn)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 629, in introspect_user_schema
[edgedb]         > json_data = await conn.parse_execute_json(
[edgedb]     edb/server/pgcon/pgcon.pyx, line 794, in parse_execute_json
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 744, in _parse_execute_json
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 688, in _parse_execute_to_buf
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 2157, in wait_for_message
[edgedb]
[edgedb]
[edgedb] ConnectionAbortedError:
[edgedb] ERROR 96328 2022-04-16T14:38:17.437 asyncio: Task exception was never retrieved
[edgedb] future: <Task finished name='Task-46' coro=<Server._on_local_database_config_change.<locals>.task() done, defined at /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py:1154> exception=ConnectionAbortedError()>
[edgedb] ---- Exception occurred: the database system is shutting down ----
[edgedb]
[edgedb] 1. edb.server.pgcon.errors.BackendError: the database system is shutting down
[edgedb]
[edgedb] ---- Traceback ----
[edgedb]
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/connpool/pool.py, line 514, in _connect
[edgedb]         > conn = await self._connect_cb(block.dbname)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 333, in _pg_connect
[edgedb]         > rv = await pgcon.connect(
[edgedb]     edb/server/pgcon/pgcon.pyx, line 322, in connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 277, in _connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 273, in edb.server.pgcon.pgcon._connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 1785, in connect
[edgedb]
[edgedb]
[edgedb] edb.server.pgcon.errors.BackendError: the database system is shutting down
[edgedb] ERROR 96328 2022-04-16T14:38:17.448 edb.server: Failed to establish a new connection to backend database: edgedb
[edgedb] ---- Exception occurred: [Errno 2] No such file or directory ----
[edgedb]
[edgedb] 1. FileNotFoundError: [Errno 2] No such file or directory
[edgedb]
[edgedb] ---- Traceback ----
[edgedb]
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/connpool/pool.py, line 514, in _connect
[edgedb]         > conn = await self._connect_cb(block.dbname)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 333, in _pg_connect
[edgedb]         > rv = await pgcon.connect(
[edgedb]     edb/server/pgcon/pgcon.pyx, line 322, in connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 253, in _connect
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2260, in create_unix_connection
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2255, in uvloop.loop.Loop.create_unix_connection
[edgedb]
[edgedb]
[edgedb] FileNotFoundError: [Errno 2] No such file or directory
[edgedb] ERROR 96328 2022-04-16T14:38:17.458 edb.server: Failed to establish a new connection to backend database: edgedb
[edgedb] ---- Exception occurred: [Errno 2] No such file or directory ----
[edgedb]
[edgedb] 1. FileNotFoundError: [Errno 2] No such file or directory
[edgedb]
[edgedb] ---- Traceback ----
[edgedb]
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/connpool/pool.py, line 514, in _connect
[edgedb]         > conn = await self._connect_cb(block.dbname)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 333, in _pg_connect
[edgedb]         > rv = await pgcon.connect(
[edgedb]     edb/server/pgcon/pgcon.pyx, line 322, in connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 253, in _connect
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2260, in create_unix_connection
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2255, in uvloop.loop.Loop.create_unix_connection
[edgedb]
[edgedb]
[edgedb] FileNotFoundError: [Errno 2] No such file or directory
[edgedb] ERROR 96328 2022-04-16T14:38:17.460 edb.server: Failed to establish a new connection to backend database: edgedb
[edgedb] ---- Exception occurred: [Errno 2] No such file or directory ----
[edgedb]
[edgedb] 1. FileNotFoundError: [Errno 2] No such file or directory
[edgedb]
[edgedb] ---- Traceback ----
[edgedb]
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/connpool/pool.py, line 514, in _connect
[edgedb]         > conn = await self._connect_cb(block.dbname)
[edgedb]     /Users/yury/Library/Application Support/edgedb/portable/1.3/lib/python3.10/site-packages/edb/server/server.py, line 333, in _pg_connect
[edgedb]         > rv = await pgcon.connect(
[edgedb]     edb/server/pgcon/pgcon.pyx, line 322, in connect
[edgedb]
[edgedb]     edb/server/pgcon/pgcon.pyx, line 253, in _connect
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2260, in create_unix_connection
[edgedb]
[edgedb]     uvloop/loop.pyx, line 2255, in uvloop.loop.Loop.create_unix_connection
[edgedb]
[edgedb]
[edgedb] FileNotFoundError: [Errno 2] No such file or directory
[edgedb] ERROR 96328 2022-04-16T14:38:17.460 edb.server: Failed to establish a new connection to backend database: edgedb
Project initialized.
To start the server run:
  edgedb instance start edgedb

edgedb project init should always start the DB when it creates an instance. Whether the instance is starting with boot or not is an entirely separate matter.

Also, I think there should be a default for this question set to "y".

elprans commented 2 years ago

Dupe of #679?

1st1 commented 2 years ago

Yes and no. The issue here is even more pronounced, because answering "no" results in our tool crashing with an error.

1st1 commented 2 years ago

@tailhook

Well, this was intentional. You are expected to run start --foreground when start-conf=manual. And also we couldn't start edgedb in any other way when we supported systemd only. So I'm not sure yet if we want to change this.

Well, if this was intentional it was a questionable decision. We have to fix this. If edgedb project init tries to run migrations automatically and fails it's a bug.

It should always run a freshly created instance. It should keep it running after edgedb prokect init is done running. The question is about "running it automatically on login", not about whether to run it now or not.

tailhook commented 2 years ago

@tailhook

Well, this was intentional. You are expected to run start --foreground when start-conf=manual. And also we couldn't start edgedb in any other way when we supported systemd only. So I'm not sure yet if we want to change this.

Well, if this was intentional it was a questionable decision. We have to fix this.

Note: it wasn't questionable decision. It was the only possible at that point.

If edgedb project init tries to run migrations automatically and fails it's a bug.

Note: migrations worked. Then EdgeDB failed to shutdown. So it looks like bug in EdgeDB shutdown code.

Actually there's nothing wrong with how it worked, except warnings from the EdgeDB. CLI didn't return failure or printed error. We can try swallowing all errors after sending a signal. But it's better to fix shutdown code.

It should always run a freshly created instance. It should keep it running after edgedb prokect init is done running. The question is about "running it automatically on login", not about whether to run it now or not.

We have to teach user how to actually run that instance. This is a good time to do that.

In my opinion, the question is more like "do you want the instance to be always running?", or even more like "do you want instance to be running in background or do you have your own script to run your app with EdgeDB?". So these are quite connected. "Run it now but don't run on login" mode looks like mostly useful for us running ad-hoc instances to test something.

And yes the question isn't specific enough. We can fix the question I think.

Update: Also note that it's --start-conf=manual, which mean "now start manually too", I think. We can ask another question at the end of wizard, but learnability is still an argument here.

1st1 commented 2 years ago

We have to teach user how to actually run that instance. This is a good time to do that.

We can ask another question at the end of wizard, but learnability is still an argument here.

No, I disagree. The installer should do the least surprising thing and have as friction-less flow as possible. Educating users shouldn't be part of it (because where do you want that education to stop?)

There's almost zero chance you'd be initializing a project and not doing anything to it in the same session. So you're requiring users to make an extra step just to educate them and this I say is a wrong way to do it.

BTW, do we have a command to make an instance start or not start on boot?

colinhacks commented 2 years ago

IMO:

instances should be started on initialization always
once stopped, they shouldn't restart unless the user runs edgedb instance start explicitly
this question isn't important enough to be part of the standard edgedb project init flow; users who want the "autostart on login" option shoujld configure that with a separate command

elprans commented 2 years ago

The problem is that the question "Do you want to start instance automatically on login?" does not indicate in any way that the instance would not be started this time. FWIW, I also find it weird that an instance I'm creating would require an extra startup step, as it's almost guaranteed that I'm creating it to work on it right now. So, we either need to reword the question and add an explanation that the instance was created in stopped state and how to start it, or fix the option to start the instance automatically.

this question isn't important enough to be part of the standard edgedb project init flow; users who want the "autostart on login" option shoujld configure that with a separate command

IIRC, we added it in response to complaints that users who create a bunch of project instances and then forget about them end up with a bunch of EdgeDB processes on their machine consuming lots of RAM. This would be less of a problem with 2.0 and on_demand compiler pool, but it's still a useful option IMO.

Hot take: I think we can eat the cake and have it if we switch to socket activation of instance servers. Both systemd and launchd have support for it. Servers can auto-shutdown after a period of inactivity and will be brought up at the first request. Thoughts?

1st1 commented 2 years ago

Hot take: I think we can eat the cake and have it if we switch to socket activation of instance servers. Both systemd and launchd have support for it. Servers can auto-shutdown after a period of inactivity and will be brought up at the first request. Thoughts?

+1. Can feel a bit flaky since it takes time for the server to start, but I think it's an OK compromise. What about Windows?

And yet... If we go with socket activation, I think we need to do it the right way. We should have a flag for local instances, something like "load policy". It can be "manual", "auto", or "boot". By default edgedb project init should set it to "auto" for the new instance. But we should have a command to also change it later to a different value, like edgedb instance load-policy foo boot or something.

tailhook commented 2 years ago

There's almost zero chance you'd be initializing a project and not doing anything to it in the same session. So you're requiring users to make an extra step just to educate them and this I say is a wrong way to do it.

There is a scenario where what you're saying is true, but current behavior is not only acceptable, but also desirable:

edgedb project init
./start_app.sh

Where the latter scripts does start EdgeDB. Note: a lot of companies have scripts to start their projects in one way or another that includes bunch of things.

Also note: this is not just a matter of interactive question. --start-conf=manual for me implies that I want to start it manually even this time.

Socket activation doesn't solve the main issue. --start-conf=manual is very often used where systemd/launchctl doesn't work.

1st1 commented 2 years ago

There is a scenario where what you're saying is true, but current behavior is not only acceptable, but also desirable:

For that ./start_app.sh could contain something like edgedb instance start proj --silent. But the script might not work at all if the user gave the instance a different name. I find this argument to be on the weak side here.

tailhook commented 2 years ago

There is a scenario where what you're saying is true, but current behavior is not only acceptable, but also desirable:

For that ./start_app.sh could contain something like edgedb instance start proj --silent.

It's more like "copy the certificates then run" or something. start is already no-op if process is already running.

But the script might not work at all if the user gave the instance a different name. I find this argument to be on the weak side here.

Yes, actual command-line is likely: edgedb project init --server-start-conf=manual --server-instance=myapp.

Now I'm thinking that maybe we need to split those:

Current --start-conf=auto -> --start-conf=eager-start (i.e. no socket activation)
Current --start-conf=manual -> --suppress-start --start-conf=eager-start
New --start-conf=manual -- do not create systemd file? (we already don't create launchd file), but start as a daemon now
New --start-conf=auto -- uses socket activation, enables the service

So that:

The use-case above is --suppress-start and doesn't care about --start-conf
--start-conf=manual doesn't care about presense/absense of systemd/launchctl, but still starts the daemon
Also --suppress-start doesn't fail when systemd/launchctl can't enable service (so that next edgedb instance start works regardless of that)

With socket activation it's also unclear whether edgedb instance stop should stop just the server or socket unit too (i.e. disable activation).

tailhook commented 2 years ago

This is fixed with socket activation #745

edgedb / edgedb-cli

broken 'edgedb project init' #710