search

tarantool / queue

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

Release all taken tasks on start #106

Closed LeonidVas closed 4 years ago

LeonidVas commented 4 years ago

If some tasks have been taken and don't released before shutdown of the tarantool instance (for example: tarantool instance has been killed) such task go to 'hung' state (noone can take the task now). So, we must release all taken tasks on start of the queue module.

Fixes #66

LeonidVas commented 4 years ago

Test fails (deploy fails) are not related with changes

Totktonada commented 4 years ago

Replace exit code in case of error from 255 to 1

Let's add 'test' prefix.

Totktonada commented 4 years ago

I put two 'FIXUP' commits.

The first one squashed duplicated testing code. It also adds the test for limfifottl.

The second one adds more logs and checking of tarantool version (as was proposed before). It also fixes the potential problem: tube = recreate_tube(tube_tuple) rewrotes upvalue tube (moduel level variable) — added local. Removed forgotten space_name variable. Moved the releasing logic into a function (the reason is mostly to skip it easily by a condition, in light of #85).

After the fixups I'm happy with the change. If you're too, please, squash the fixups into the base commit and I'll proceed with the patchset.

LeonidVas commented 4 years ago

About common tests. I think about it, but I'm disagree with proposed concept. If look at drivers tests it have many copy-paste code. My point of view: "We must to have some common tests which will be run for all/some drivers from list". And this is not related to the patch. limfifottl don't need the test, because it is a wrap around fifottl.

Totktonada commented 4 years ago

We must to have some common tests which will be run for all/some drivers from list

The test case should be either referenced from a certail test file (like 020-fifottl.t) or it should be obvious from names that certain part of cases is in a separate file (say, ddd-driver_api.t).

We discussed it more with Leonid and he want to keep current way for now and make deduplication separately. I would do it like proposed above, but this way is okay too.

limfifottl don't need the test, because it is a wrap around fifottl.

It is implementation detail, but we test public API of a driver. So I would left the test here.

LeonidVas commented 4 years ago

Updated

LeonidVas commented 4 years ago

limfifottl does not duplicate fifottl tests. So, let's continue to go that way