sqlite errors

[Rhett Garber]

I started using gearman in production last night for the first time. I
had a few crashes that seem to be due to sqlite issues.

This was in the server logs:

FATAL _sqlite_add:insert error: SQL logic error or missing database
FATAL _sqlite_add:finalize error: column unique_key is not unique

The behavior from the client side was a dropped connection after
trying to queue a background job.

I'm running two servers and it happened on both of them. They are both
still running.

The sqlite db is also empty now in both cases.
Some jobs were successfully process. I see about 40 successful jobs
and about 10 sets of the above error
message.

Any ideas ?

Rhett

[Vick Khera]

On Tue, Dec 22, 2009 at 3:19 PM, Rhett Garber <rhe...@gmail.com> wrote:
> FATAL _sqlite_add:finalize error: column unique_key is not unique
>

what client are you using to submit the jobs? for the old perl
client, you have to pass a unique ID to it for background jobs because
it doesn't make one for you like the C client does (at least in Perl).

[Rhett Garber]

I'm using python-gearman (http://samuelks.com/python-gearman/) version 1.4.1

Against gearmand 0.10

Rhett

[Rhett Garber]

As an update I found a description of this error here:
http://www.vijaykiran.com/sqlite3-exception-sql-logic-error-or-missing-database/

Seems to indicate that there is something wrong with the INSERT query.

This happened exactly 7 times on both servers, which I'm hoping means
that the client was smart enough to try to run
this on each server, so it's a sort of 'query of death' for libsqlite.

An explanation of where the error messages comes from is found here in
queue_libsqlite3.c _sqlite_add() :

if (sqlite3_step(sth) != SQLITE_DONE)
{
GEARMAN_SERVER_ERROR_SET(server, "_sqlite_add", "insert error: %s",
sqlite3_errmsg(queue->db));
if (sqlite3_finalize(sth) != SQLITE_OK )
{
GEARMAN_SERVER_ERROR_SET(server, "_sqlite_add", "finalize error: %s",
sqlite3_errmsg(queue->db));
}

return GEARMAN_QUEUE_ERROR;
}

It seems to me that this indicates a bug. Perhaps I'm doing something
strange to cause the client to cause this situation, but
I can't imagine having SQL errors is ever appropriate. So I guess the
next step is to add enough debug logging to find out what is
actually wrong about this query.

I'd appreciate if anyone else has any insight or could tell me if I'm
obviously missing something.

Also, I'm having trouble understanding how these queue functions get
called. I figure there must be some levels of abstraction here that
I don't understand, but I'd like to see what happens to the caller
when that GEARMAN_QUEUE_ERROR gets returned.

Thanks,

Rhett

[Cory Bennett]

I have been using the sqlite3 with 0.9 without problems for several months now.
The link you pointed to seems to indicate that that a NULL was inserted
into a non NULL field. I am not sure how/why that would happen.

Have you tried running gearmand with -v -v -v -v. (Not sure how many
-v's are useful)
I think you should be able to see the sql statements when running with
high verbosity.

-Cory

[Rhett Garber]

Yeah actually I just checked the sqlite source for that error message
and basically it's just the generic
"We don't know what happened" message (SQLITE_ERROR)

If I run with a bunch of -v's I eventually get output like this:
CRAZY [ 0] ::9669:314b:0:0:41751 Watching POLLIN
INFO [ 0] ::9669:314b:0:0:41751 Connected
CRAZY [ 0] ::9669:314b:0:0:41751 Ready POLLIN
DEBUG [ 0] ::9669:314b:0:0:41751 Received SUBMIT_JOB_BG
DEBUG sqlite add:
CRAZY sqlite query: BEGIN TRANSACTION
CRAZY sqlite query: INSERT INTO gearman_queue
(priority,unique_key,function_name,data) VALUES (?,?,?,?)
CRAZY sqlite query: COMMIT
DEBUG sqlite flush

But it doesn't actually have the values so that's not very helpful.
I'll probably need to patch gearmand to actually report the
unique key, function and data (or maybe just the sizes ?).

Rhett

[Eric Day]

In the client, are you specifying a unique job ID for each job
you submit? The persistent queue requires a unique ID to be given,
otherwise you would only be allowed to have one job in the queue at
a time (a job with a NULL unique ID).

The Gearman APIs based on the C library automatically generate a uuid
for you if no ID is given, most of the non-C lib based APIs you need
to manually specify one though.

-Eric

[Rhett Garber]

Whoa... that's interesting.

Looks like the python client doesn't generate a unique key for you
automatically so that could be
my issue.

Was it a conscious decision to not have unique keys automatically
generated all the time ?

What do most people do ? Just uuid ?

Sounds like it might be a good idea to provide some sort of reasonable
error message back to the client in this case.
Thoughts ?

Rhett

[Monty Taylor]

There's also another python lib that's based on the C-lib:

http://pypi.python.org/pypi/python-libgearman

Since it uses the C library, it has the same behaviors, such as
generating a unique key for you.

Monty

[Eric Day]

The unique ID should always be client generated, so it's up to the
client APIs to manage that. The error message could be improved upon,
I agree. :)

Some folks just need something unique, so UUID is fine. Some folks
use this for job coalescing to avoid extra work. For example, if
you have an image resize service, your unique may be the image path
(or image ID) so it only gets run once at any given time. The server
"merges" jobs that have the same unique ID.

-Eric

[Rhett Garber]

Ok, so it sounds like the *bug* is that if you are using libsqlite for
persistence,
it doesn't coalesce properly because it's getting some sort of
integrity error before it
has a chance to find out it doesn't need to do something.

If that's true then it's not really an error message issue, but I
should have had a much more subtle bug
where I'd lose jobs that overlap because it thinks they are the same.

So:
1. The python client should auto-generate unique ids (for usability
more than anything else)
2. queue_libsqlite needs to be fixed to maintain unique id
coalescing behavior (rather than generate a strange error and drop the
client)

Rhett

[Eric Day]

Well, it's not really a bug in the SQLite queue. The Gearman server
is what does the coalescing, not the queues. When the job server
sees an empty unique ID or a unique ID of '-', it does not try to
coalesce. This means there are multiple valid jobs with a empty unique
ID, and the SQLite queue errors because it has a unique constraint
on that column. So, there are a few places this could be "fixed",
but the easiest for now is to just specify this in the client.

I didn't want to automatically do this for blank ID's in the job server
since this would be a bit of extra processing. It really belongs in
the client. At the very least, error reporting could be improved in
the server.

-Eric

[Monty Taylor]

Rhett Garber wrote:
> Ok, so it sounds like the *bug* is that if you are using libsqlite for
> persistence,
> it doesn't coalesce properly because it's getting some sort of
> integrity error before it
> has a chance to find out it doesn't need to do something.
>
> If that's true then it's not really an error message issue, but I
> should have had a much more subtle bug
> where I'd lose jobs that overlap because it thinks they are the same.
>
> So:
> 1. The python client should auto-generate unique ids (for usability
> more than anything else)

Again... there is more than one python client lib for Gearman. If you
use python-libgearman it possesses this property.

[Rhett Garber]

>>   1. The python client should auto-generate unique ids (for usability
>> more than anything else)
>
> Again... there is more than one python client lib for Gearman. If you
> use python-libgearman it possesses this property.

Right... should have been more specific.
Is the C based python library more popular ?

Rhett

[Monty Taylor]

Popularity I don't know about. The C-based one is based on the client
lib. I know of people using it. *

The pure python one is pure python. I also know of people using it. It's
been around longer.

Monty

* disclaimer: author of the C-based one

[Rhett Garber]

Yeah actually it seems like an issue with the coalescing now that i've
looked at that part of the code.
It special cases "-" to generate a hash based on the data rather than
relying on a unique key and then
lookup the job then. (this seems a little dangerous since it doesn't
include the function name, but whatever..)

gearman_server_job_st *
gearman_server_job_add(gearman_server_st *server, const char *function_name,
size_t function_name_size, const char *unique,
size_t unique_size, const void *data, size_t data_size,
gearman_job_priority_t priority,
gearman_server_client_st *server_client,
gearman_return_t *ret_ptr)
....
if (unique_size == 1 && *unique == '-')
{
if (data_size == 0)
{
key= 0;
server_job= NULL;
}
else
{
/* Look up job via unique data when unique = '-'. */
key= _server_job_hash(data, data_size);
server_job= _server_job_get_unique(server, key, server_function, data,
data_size);
}

So I guess the issue is that the hash lookup fails (because the data
is different) so it thinks it's cool to store the
job in the db. So if sqlite used the hash key as the primary key
rather than 'unique key' it would work properly.

I'm trying to figure how these test cases work to see if I can
reproduce this. Maybe tomorrow.

Rhett

[Brian Moon]

> What do most people do ? Just uuid ?

In the PEAR lib, md5 of the json value of the arguments is used so that
duplicate jobs are not run.

Brian.

[John Ewart]

In the C# library I'm working on, if an ID is not provided it will generate a GUID (effectively C#'s implementation of UUID.)

-John

[Rhett Garber]

I've submitted the following patches:
Restrict usage of blank ids in server
https://code.launchpad.net/~rhettg/gearmand/no-unique/+merge/16603

And in the python client:
Adds uuid unique key by default (and documents behavior)
http://github.com/rhettg/python-gearman/tree/default-unique

Let me know what you guys think.

Rhett