tarantool / queue

Create task queues, add and take jobs, monitor failed tasks
Other
237 stars 52 forks source link

require('queue') fails in read_only mode #122

Closed printercu closed 4 years ago

printercu commented 4 years ago
$ tarantool
box.cfg{}
os.exit()

$ tarantool
require('queue')
box.cfg{read_only = true}
E> ER_READONLY: Can't modify data because this instance is in read-only mode.
os.exit()

$ tarantool
box.cfg{read_only = true}
require('queue')
E> ER_READONLY: Can't modify data because this instance is in read-only mode.
os.exit()
printercu commented 4 years ago

This makes it impossible to use queue in cartridge, because it starts instances in read_only mode.

Totktonada commented 4 years ago

NB: Aside of implementing postponing of queue initialization until box will leave read-only mode we need to verify whether we give proper errors when a write operation (say, take()) is requested in the read only mode.

printercu commented 4 years ago

Do you think that tarantool's error Can't modify data because this instance is in read-only mode. is not enough?

Totktonada commented 4 years ago

The message is okay.

LeonidVas commented 4 years ago

It is required to understand the DoD of the issue ( @Totktonada ). I can propose the following solutions:

  1. The "lazy" start, delayed until read_only is set to false. The switching true -> false -> true is not supported.
  2. After starting the queue with box.cfg{read_only=true} only "temporary" queues work (but new temporary queues can't be created!!!). Persistent queues became available after read_only is set to false. The switching true -> false -> true is not supported.
  3. Previous with toggle support true -> false -> true. For this solution, a stop method must be implemented in the drivers for free the resources(for example, stop background fibers in fifottl).

The reasons, why in read_only mode the persistent queues in unavailable:

printercu commented 4 years ago

Explicit queue.init() would be much better. require should be idempotent and do the same things no matter box is configured or not. Sure, this will be not backward-compatible but much more expected behavior.

LeonidVas commented 4 years ago

Explicit queue.init() would be much better. require should be idempotent and do the same things no matter box is configured or not. Sure, this will be not backward-compatible but much more expected behavior.

Hi. If I understand correctly, your proposal doesn't solve the problems with toggling and persistent queues. Are you suggesting something like: "If you are trying to execute init(), you should do box.cfg{read_only = false} before"?

Breaking backward compatibility is a last resort. I think this is not our way.

printercu commented 4 years ago

Like every other DDL (ex., space creation) it would be wrapped with if not box.cfg.read_only then (or if options.is_master then in cartridge).

Totktonada commented 4 years ago

@printercu Would lazy start (1st option of the proposed by Leonid) resolve the problem with cartridge?

I would implement lazy starting here and return later to switching from/to read_only mode at any time if there will be demand.

printercu commented 4 years ago

Well, maybe. I haven't seen in the other rocks patching box.cfg after it has been called, and don't know if there could be any pitfalls.