Browse Source

Additional cleanup for task job_handle checking.

Brian Aker 13 years ago
parent
commit
2b42735785

+ 4 - 1
docs/libgearman/gearman_client_job_status.rst

@@ -10,6 +10,7 @@ SYNOPSIS
 
 .. c:function:: gearman_return_t gearman_client_job_status(gearman_client_st *client, gearman_job_handle_t job_handle, bool *is_known, bool *is_running, uint32_t *numerator, uint32_t *denominator);
 
+Link with -lgearman
 
 -----------
 DESCRIPTION
@@ -27,6 +28,8 @@ RETURN VALUE
 If the arguments is_known and is_running are omitted then :c:func:`gearman_client_job_status()` will return :c:type:`GEARMAN_JOB_EXISTS` if the :c:type:`gearman_job_handle_t` is known on the server, and
 :c:type:`GEARMAN_IN_PROGRESS` if it is running. Knowing whether the job is running or not has higher precedence. :c:func:`gearman_continue()` can be used for loops where you want to exit from the loop once the job has been completed. 
 
+.. warning:: 
+  For loops you should always check :c:type:`gearman_return_t` with :c:func:`gearman_continue()` even if you specifiy the argument is_known or is_running. A non-blocking IO call can return something other then :c:type:`GEARMAN_SUCCESS`, in some cases you will want to treat those values not as errors.
 
 
 ----
@@ -38,5 +41,5 @@ To find out more information please check:
 `https://launchpad.net/gearmand <https://launchpad.net/gearmand>`_
 
 .. seealso::
-  :manpage:`gearmand(8)` :manpage:`libgearman(3)` :manpage:`gearman_client_st(3)` :manpage:`gearman_job_handle_t(3)`
+  :manpage:`gearmand(8)` :manpage:`libgearman(3)` :manpage:`gearman_client_st(3)` :manpage:`gearman_job_handle_t(3)` :manpage:`gearman_continue(3`
 

+ 7 - 1
docs/man/gearman_client_job_status.3

@@ -37,6 +37,8 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .TP
 .B gearman_return_t gearman_client_job_status(gearman_client_st *client, gearman_job_handle_t job_handle, bool *is_known, bool *is_running, uint32_t *numerator, uint32_t *denominator);
 .UNINDENT
+.sp
+Link with \-lgearman
 .SH DESCRIPTION
 .sp
 \fBgearman_client_job_status()\fP is used to find the status of a job via its \fBgearman_job_handle_t\fP. The arguments is_known, is_running, numerator, and denominator are all optional. Unless a Gearman worker has sent numerator/denominator via \fBgearman_job_send_status()\fP, those values will be zero.
@@ -47,13 +49,17 @@ If the arguments is_known and is_running are ommitted then the function returns
 \fBgearman_return_t\fP will be returned. \fBGEARMAN_SUCCESS\fP will be returned upon success of the status being updated.
 If the arguments is_known and is_running are omitted then \fBgearman_client_job_status()\fP will return \fBGEARMAN_JOB_EXISTS\fP if the \fBgearman_job_handle_t\fP is known on the server, and
 \fBGEARMAN_IN_PROGRESS\fP if it is running. Knowing whether the job is running or not has higher precedence. \fBgearman_continue()\fP can be used for loops where you want to exit from the loop once the job has been completed.
+.IP Warning
+.
+For loops you should always check \fBgearman_return_t\fP with \fBgearman_continue()\fP even if you specifiy the argument is_known or is_running. A non\-blocking IO call can return something other then \fBGEARMAN_SUCCESS\fP, in some cases you will want to treat those values not as errors.
+.RE
 .SH HOME
 .sp
 To find out more information please check:
 \fI\%https://launchpad.net/gearmand\fP
 .SH SEE ALSO
 .sp
-\fIgearmand(8)\fP \fIlibgearman(3)\fP \fIgearman_client_st(3)\fP \fIgearman_job_handle_t(3)\fP
+\fIgearmand(8)\fP \fIlibgearman(3)\fP \fIgearman_client_st(3)\fP \fIgearman_job_handle_t(3)\fP \fIgearman_continue(3\fP
 .RE
 .SH AUTHOR
 Data Differential http://datadifferential.com/

+ 8 - 6
examples/reverse_client_bg.cc

@@ -144,9 +144,9 @@ int main(int args, char *argv[])
   std::cout << "Background Job Handle=" << gearman_task_job_handle(task) << std::endl;
 
   int exit_code= EXIT_SUCCESS;
+  bool is_known;
   do
   {
-    bool is_known;
     bool is_running;
     uint32_t numerator;
     uint32_t denominator;
@@ -154,10 +154,15 @@ int main(int args, char *argv[])
     ret= gearman_client_job_status(&client, gearman_task_job_handle(task),
                                    &is_known, &is_running,
                                    &numerator, &denominator);
-    if (gearman_failed(ret))
+    if (gearman_continue(ret)) // Non-blocking event occurred, try again
+    {
+      continue;
+    }
+    else if (gearman_failed(ret))
     {
       std::cerr << gearman_client_error(&client) << std::endl;
       exit_code= EXIT_FAILURE;
+      break;
     }
 
     std::cout << std::boolalpha 
@@ -165,10 +170,7 @@ int main(int args, char *argv[])
       << ", Running=" << is_running
       << ", Percent Complete=" << numerator << "/" <<  denominator << std::endl;
 
-    if (not is_known)
-      break;
-
-  } while (gearman_task_is_running(task));
+  } while (is_known);
 
   gearman_client_free(&client);
 

+ 8 - 7
examples/reverse_client_epoch.cc

@@ -139,9 +139,9 @@ int main(int args, char *argv[])
   std::cout << "Background Job Handle=" << gearman_task_job_handle(task) << std::endl;
 
   int exit_code= EXIT_SUCCESS;
+  bool is_known;
   do
   {
-    bool is_known;
     bool is_running;
     uint32_t numerator;
     uint32_t denominator;
@@ -149,10 +149,15 @@ int main(int args, char *argv[])
     ret= gearman_client_job_status(&client, gearman_task_job_handle(task),
                                    &is_known, &is_running,
                                    &numerator, &denominator);
-    if (gearman_failed(ret))
+    if (gearman_continue(ret)) // Non-blocking event occurred, try again
+    {
+      continue;
+    }
+    else if (gearman_failed(ret))
     {
       std::cerr << gearman_client_error(&client) << std::endl;
       exit_code= EXIT_FAILURE;
+      break;
     }
 
     std::cout << std::boolalpha 
@@ -160,11 +165,7 @@ int main(int args, char *argv[])
       << ", Running=" << is_running
       << ", Percent Complete=" << numerator << "/" <<  denominator << std::endl;
 
-    if (not is_known)
-      break;
-
-    sleep(1);
-  } while (gearman_task_is_running(task));
+  } while (is_known);
 
   gearman_client_free(&client);
 

+ 6 - 1
libgearman/client.cc

@@ -123,7 +123,7 @@ gearman_client_st *gearman_client_clone(gearman_client_st *client,
 
   client= _client_allocate(client, true);
 
-  if (client == NULL)
+  if (not client)
   {
     return client;
   }
@@ -531,6 +531,11 @@ gearman_return_t gearman_client_job_status(gearman_client_st *client,
 
   do {
     ret= gearman_client_run_tasks(client);
+    
+    // If either of the following is ever true, we will end up in an
+    // infinite loop
+    assert(ret != GEARMAN_IN_PROGRESS and ret != GEARMAN_JOB_EXISTS);
+
   } while (gearman_continue(ret));
 
   if (gearman_success(ret))

+ 5 - 2
tests/basic.cc

@@ -37,6 +37,7 @@
 
 #include <config.h>
 #include <cstring>
+#include <cassert>
 
 #include <libgearman/gearman.h>
 
@@ -162,12 +163,14 @@ test_return_t queue_add(void *object)
   rc= gearman_client_do_background(&client, test->worker_function_name(), NULL, 
                                    gearman_literal_param("background_payload"),
                                    job_handle);
-  test_true_got(rc == GEARMAN_SUCCESS, gearman_strerror(rc));
+  test_compare(GEARMAN_SUCCESS, rc);
   test_truth(job_handle[0]);
 
   do {
     rc= gearman_client_job_status(client_ptr, job_handle, NULL, NULL, NULL, NULL);
-  } while (gearman_continue(rc));
+    test_true(rc != GEARMAN_IN_PROGRESS);
+  } while (gearman_continue(rc) and rc != GEARMAN_JOB_EXISTS); // We need to exit on these values since the job will never run
+  test_true(rc == GEARMAN_JOB_EXISTS or rc == GEARMAN_SUCCESS);
 
   gearman_client_free(&client);
 

+ 4 - 3
tests/client_test.cc

@@ -586,6 +586,7 @@ static test_return_t gearman_client_job_status_test(void *object)
                    gearman_client_do_background(client, worker_function, NULL, gearman_string_param(value), job_handle), 
                    gearman_client_error(client));
 
+  gearman_return_t ret;
   bool is_known;
   do
   {
@@ -594,9 +595,9 @@ static test_return_t gearman_client_job_status_test(void *object)
     uint32_t denominator;
 
     test_compare_got(GEARMAN_SUCCESS,
-                     gearman_client_job_status(client, job_handle, &is_known, &is_running, &numerator, &denominator),
+                     ret= gearman_client_job_status(client, job_handle, &is_known, &is_running, &numerator, &denominator),
                      gearman_client_error(client));
-  } while (is_known);
+  } while (gearman_continue(ret) and is_known);
 
   return TEST_SUCCESS;
 }
@@ -647,7 +648,7 @@ static test_return_t background_failure_test(void *object)
     rc= gearman_client_job_status(client, job_handle, &is_known, &is_running,
                                   &numerator, &denominator);
     test_true(is_known == true and is_running == false and numerator == 0 and denominator == 0);
-  } while (gearman_continue(rc));
+  } while (gearman_continue(rc)); // We do not test for is_known since the server will keep the job around until a worker comes along
   test_compare(GEARMAN_SUCCESS, rc);
 
   return TEST_SUCCESS;

+ 8 - 3
tests/sqlite.am

@@ -29,17 +29,22 @@ tests_sqlite_test_SOURCES= \
 			   tests/sqlite_test.cc
 tests_sqlite_test_LDADD= ${CLIENT_LDADD}
 
-test-sqlite-queue:
+sqlite-nuke: 
+	@rm -f tests/gearman.sql
+	@rm -f tests/gearman.sql-journal
+	@rm -f tests/gearmand.log*
+
+test-sqlite-queue: sqlite-nuke
 if HAVE_LIBSQLITE3
 	$(SQLITE_TEST) $(ARG1) $(ARG2)
 endif
 
-gdb-sqlite:
+gdb-sqlite: sqlite-nuke
 if HAVE_LIBSQLITE3
 	$(LIBTOOL) --mode=execute gdb tests/sqlite_test
 endif
 
-valgrind-sqlite:
+valgrind-sqlite: sqlite-nuke
 if HAVE_LIBSQLITE3
 	$(VALGRIND_COMMAND) $(SQLITE_TEST) $(ARG1) $(ARG2)
 endif