[Ur] missing transactions (was Code review request)

[Sergey Mironov]

Hi again! Looks like there are no people around who are brave enough
to look into my code :)) But that's alright: after deep debugging I've
fixed the segfaults (my mistake). And it is not the end of story:

I've run a stress testing and it shows that some database transactions
just disappear . Investigation shows an interesting point in uw_commit
(see the code below). The code says that it is possible to return from
the [uw_commit] function after calling all the
transactionals[i].commit handlers (return 1 after db_commit). I think
it is clearly a bug, since the user applications (like urweb-callback)
think that everything is OK at this point. I've done some testing and
it shows that this returns really happen from time to time under high
load.

So there are two questions:

 1. What should application do if database commit fails? Probably, the
answer is - report the error to client.
 2. How to notify the ctx->transactionals about successful db_commit?
Looks like we can't do that at the moment.  Here I suggest rewriting
the code: call NULL-rolllbacked commits _after_ the db_commit point
(and forbid raising errors for such handlers). This way we can really
handle the non-transactional actions.

Please, comment.

Regards,
Sergey


--

urweb.c   line 3291

  for (i = ctx->used_transactionals-1; i >= 0; --i)
    if (ctx->transactionals[i].rollback != NULL)
      if (ctx->transactionals[i].commit) {
        ctx->transactionals[i].commit(ctx->transactionals[i].data);
        if (uw_has_error(ctx)) {
          uw_rollback(ctx, 0);
          return 0;
        }
      }

  for (i = ctx->used_transactionals-1; i >= 0; --i)
    if (ctx->transactionals[i].rollback == NULL)
      if (ctx->transactionals[i].commit) {
        ctx->transactionals[i].commit(ctx->transactionals[i].data);
        if (uw_has_error(ctx)) {
          uw_rollback(ctx, 0);
          return 0;
        }
      }

  if (ctx->transaction_started) {
    int code = ctx->app->db_commit(ctx);

    if (code) {
      if (code == -1)
        return 1;  // <--- HERE !

      for (i = ctx->used_transactionals-1; i >= 0; --i)
        if (ctx->transactionals[i].free)
          ctx->transactionals[i].free(ctx->transactionals[i].data, 0);

      uw_set_error_message(ctx, "Error running SQL COMMIT");
      return 0;
    }
  }



2014-04-07 0:50 GMT+04:00 Sergey Mironov <grrwlf at gmail.com>:
> Probably I should mention this thing: the notifiers::init() from
> CallbackFFI.cpp:377 is called by the `task initialize' in the
> Callback.ur
>
> https://github.com/grwlf/urweb-callback/blob/devel/Callback.ur#L18
>
>
> Sergey