From a24c2bdaf85c3d4eef19783e95b11d1cf15add09 Mon Sep 17 00:00:00 2001 From: Adam Chlipala Date: Thu, 12 Dec 2013 17:42:48 -0500 Subject: Start SQL transactions as read-only when possible, based on conservative program analysis --- src/c/fastcgi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/c/fastcgi.c') diff --git a/src/c/fastcgi.c b/src/c/fastcgi.c index 9e3c8d7e..d6d2391d 100644 --- a/src/c/fastcgi.c +++ b/src/c/fastcgi.c @@ -632,8 +632,7 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - if (uw_get_app(ctx)->db_begin(ctx)) - uw_error(ctx, FATAL, "Error running SQL BEGIN"); + uw_ensure_transaction(ctx); uw_get_app(ctx)->expunger(ctx, cli); uw_commit(ctx); } -- cgit v1.2.3 From fac05ae0a6d7d080436c953d2085e137d75c5796 Mon Sep 17 00:00:00 2001 From: Adam Chlipala Date: Mon, 23 Dec 2013 15:59:17 +0000 Subject: Proper handling of serialization failures during SQL COMMIT --- include/urweb/urweb_cpp.h | 3 ++- src/c/cgi.c | 7 ++++--- src/c/fastcgi.c | 7 ++++--- src/c/http.c | 7 ++++--- src/c/request.c | 6 ++++-- src/c/urweb.c | 29 ++++++++++++++++++++++------- src/postgres.sml | 18 +++++++++++++++++- 7 files changed, 57 insertions(+), 20 deletions(-) (limited to 'src/c/fastcgi.c') diff --git a/include/urweb/urweb_cpp.h b/include/urweb/urweb_cpp.h index 8dfffdf9..248e54e4 100644 --- a/include/urweb/urweb_cpp.h +++ b/include/urweb/urweb_cpp.h @@ -40,7 +40,8 @@ failure_kind uw_begin(struct uw_context *, char *path); void uw_ensure_transaction(struct uw_context *); failure_kind uw_begin_onError(struct uw_context *, char *msg); void uw_login(struct uw_context *); -void uw_commit(struct uw_context *); +int uw_commit(struct uw_context *); +// ^-- returns nonzero if the transaction should be restarted int uw_rollback(struct uw_context *, int will_retry); __attribute__((noreturn)) void uw_error(struct uw_context *, failure_kind, const char *fmt, ...); diff --git a/src/c/cgi.c b/src/c/cgi.c index c9ec744a..f1482589 100644 --- a/src/c/cgi.c +++ b/src/c/cgi.c @@ -134,9 +134,10 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - uw_commit(ctx); + do { + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/fastcgi.c b/src/c/fastcgi.c index d6d2391d..bbda0f57 100644 --- a/src/c/fastcgi.c +++ b/src/c/fastcgi.c @@ -632,9 +632,10 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - uw_commit(ctx); + do { + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/http.c b/src/c/http.c index c57740e9..9050aaf4 100644 --- a/src/c/http.c +++ b/src/c/http.c @@ -447,9 +447,10 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - uw_commit(ctx); + do { + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/request.c b/src/c/request.c index 5973d979..b925cc3c 100644 --- a/src/c/request.c +++ b/src/c/request.c @@ -116,8 +116,10 @@ static void *periodic_loop(void *data) { return NULL; } while (r == UNLIMITED_RETRY || (r == BOUNDED_RETRY && retries_left > 0)); - if (r != FATAL && r != BOUNDED_RETRY) - uw_commit(ctx); + if (r != FATAL && r != BOUNDED_RETRY) { + if (uw_commit(ctx)) + r = UNLIMITED_RETRY; + } sleep(p->pdic.period); }; diff --git a/src/c/urweb.c b/src/c/urweb.c index 3082f110..57f57694 100644 --- a/src/c/urweb.c +++ b/src/c/urweb.c @@ -3253,13 +3253,13 @@ static char *find_sig(char *haystack) { return s; } -void uw_commit(uw_context ctx) { +int uw_commit(uw_context ctx) { int i; char *sig; if (uw_has_error(ctx)) { uw_rollback(ctx, 0); - return; + return 0; } for (i = ctx->used_transactionals-1; i >= 0; --i) @@ -3268,7 +3268,7 @@ void uw_commit(uw_context ctx) { ctx->transactionals[i].commit(ctx->transactionals[i].data); if (uw_has_error(ctx)) { uw_rollback(ctx, 0); - return; + return 0; } } @@ -3278,13 +3278,26 @@ void uw_commit(uw_context ctx) { ctx->transactionals[i].commit(ctx->transactionals[i].data); if (uw_has_error(ctx)) { uw_rollback(ctx, 0); - return; + return 0; } } - if (ctx->transaction_started && ctx->app->db_commit(ctx)) { - uw_set_error_message(ctx, "Error running SQL COMMIT"); - return; + if (ctx->transaction_started) { + int code =ctx->app->db_commit(ctx); + + if (code) { + if (code == -1) { + uw_rollback(ctx, 1); + return 1; + } + + 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; + } } for (i = 0; i < ctx->used_deltas; ++i) { @@ -3390,6 +3403,8 @@ void uw_commit(uw_context ctx) { } while (sig); } } + + return 0; } diff --git a/src/postgres.sml b/src/postgres.sml index 272097e7..8cfa5f48 100644 --- a/src/postgres.sml +++ b/src/postgres.sml @@ -438,7 +438,23 @@ fun init {dbstring, prepared = ss, tables, views, sequences} = newline, newline, string "if (PQresultStatus(res) != PGRES_COMMAND_OK) {", - box [string "PQclear(res);", + box [string "if (!strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), \"40001\")) {", + box [newline, + string "PQclear(res);", + newline, + string "return -1;", + newline], + string "}", + newline, + string "if (!strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), \"40P01\")) {", + box [newline, + string "PQclear(res);", + newline, + string "return -1;", + newline], + string "}", + newline, + string "PQclear(res);", newline, string "return 1;", newline], -- cgit v1.2.3 From aea9e6db8a7a72dd555913a38cb893d247c3c09e Mon Sep 17 00:00:00 2001 From: Adam Chlipala Date: Wed, 25 Dec 2013 13:11:43 -0500 Subject: Tweaking handling of database transactions --- src/c/cgi.c | 9 +++++---- src/c/fastcgi.c | 9 +++++---- src/c/http.c | 9 +++++---- src/c/urweb.c | 8 ++++++-- 4 files changed, 21 insertions(+), 14 deletions(-) (limited to 'src/c/fastcgi.c') diff --git a/src/c/cgi.c b/src/c/cgi.c index f1482589..539b83c2 100644 --- a/src/c/cgi.c +++ b/src/c/cgi.c @@ -134,10 +134,11 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - do { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + + if (uw_commit(ctx)) + uw_error(ctx, UNLIMITED_RETRY, "Rerunning expunge transaction"); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/fastcgi.c b/src/c/fastcgi.c index bbda0f57..5c80d3ae 100644 --- a/src/c/fastcgi.c +++ b/src/c/fastcgi.c @@ -632,10 +632,11 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - do { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + + if (uw_commit(ctx)) + uw_error(ctx, UNLIMITED_RETRY, "Rerunning expunge transaction"); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/http.c b/src/c/http.c index 9050aaf4..17833ce1 100644 --- a/src/c/http.c +++ b/src/c/http.c @@ -447,10 +447,11 @@ void uw_copy_client_data(void *dst, void *src) { } void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) { - do { - uw_ensure_transaction(ctx); - uw_get_app(ctx)->expunger(ctx, cli); - } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1)); + uw_ensure_transaction(ctx); + uw_get_app(ctx)->expunger(ctx, cli); + + if (uw_commit(ctx)) + uw_error(ctx, UNLIMITED_RETRY, "Rerunning expunge transaction"); } void uw_post_expunge(uw_context ctx, void *data) { diff --git a/src/c/urweb.c b/src/c/urweb.c index ab6e5b1f..af5cc461 100644 --- a/src/c/urweb.c +++ b/src/c/urweb.c @@ -3225,7 +3225,11 @@ int uw_rollback(uw_context ctx, int will_retry) { if (ctx->transactionals[i].free) ctx->transactionals[i].free(ctx->transactionals[i].data, will_retry); - return (ctx->app && ctx->transaction_started) ? ctx->app->db_rollback(ctx) : 0; + if (ctx->app && ctx->transaction_started) { + ctx->transaction_started = 0; + return ctx->app->db_rollback(ctx); + } else + return 0; } static const char begin_xhtml[] = "\n\n"; @@ -3461,8 +3465,8 @@ void uw_prune_clients(uw_context ctx) { prev->next = next; else clients_used = next; - uw_reset(ctx); while (fk == UNLIMITED_RETRY) { + uw_reset(ctx); fk = uw_expunge(ctx, c->id, c->data); if (fk == UNLIMITED_RETRY) printf("Unlimited retry during expunge: %s\n", uw_error_message(ctx)); -- cgit v1.2.3