[Support] BukuBot issue with database lock error

[sjehuda]

Please pardon for not posting this at #719 (the thread is locked)

I have an issue with buku which sends a database lock error ERROR database is locked.

See https://git.xmpp-it.net/sch/BukuBot/issues/7

This occurs upon a deletion or several deletions, either one by one or at the same time.

The code that is reponsible for deletion is:

                case _ if message_lowercase.startswith('del '):
                    val = message_lowercase[4:]
                    ixs = message_lowercase[4:].split(',')
                    message_body = ''
                    ixs_accepted = []
                    ixs_rejected = []
                    for ix in ixs:
                        try:
                            ixs_accepted.append(int(ix))
                        except:
                            ixs_rejected.append(ix)
                    ixs_accepted.sort(reverse=True)
                    for ix in ixs_accepted:
                        bookmark = bookmarks_db.get_rec_by_id(ix)
                        if bookmark: message_body += Chat.format_message(bookmark, extended=True)
                        result = bookmarks_db.delete_rec(ix)
                        if result:
                            message_body += '\n*Bookmark has been deleted.*\n'
                        else:
                            message_body += '*Index {} does not exist.*\n'.format(ix)
                        message_body += '\n'
                    message_body += '*Rejected indexes:* {} (ID must be a number)'.format(', '.join(ixs_rejected))

[LeXofLeviafan]

Can you reproduce this manually (in Python prompt or in a standalone script)? If yes, please describe necessary conditions to reproduce it (e.g. minimum initial number of records, and a sequence of indices to be removed by executing .delete_rec(idx) that leads to an error).

I haven't managed to reproduce it via trivial imitation (even in a loop); so it seems to me like this might be caused by actual attempts for concurrent access (possibly due to the bot being run in multiple threads or something of the sort).

…Also, here's some feedback on the code :sweat_smile:

1. If there's no bookmark with the given ID, the deletion should probably be skipped altogether, no? (And that should also be the condition to determine whether the "index doesn't exist"; otherwise the message should be something in the lines of "deletion failed".)
2. When performing a batch deletion, it's probably a good idea to use delay_commit=True parameter (and then running .conn.commit() manually after the completion). Alternatively, .delete_resultset() could be used (though it doesn't appear to report on deletion failures… mostly because it expects fresh DB search results as input).
3. Check for indices to be positive integers (and describe them as such in the output: neither -4 nor 3.14 are valid, and 0 is valid input but it's actually equivalent to calling .delete_rec_all()).
4. Also check for duplicate indices – those would result in buggy behaviour when evaluated one by one.
5. …Actually I'd say, just call .get_rec_all_by_ids() then pass the result to .delete_resultset(). (…After the fix is merged :sweat_smile:)
6. You should probably either delete the val = line, or use the assigned value at least once.

[sjehuda]

Can you reproduce this manually (in Python prompt or in a standalone script)?

I have only tried with (Pdb) (breakpoint()) and I have failed to reproduce it.

I have changed the code:

                case _ if message_lowercase.startswith('del '):
                    ixs = message_lowercase[4:].split(',')
                    ixs = set(ixs)
                    message_body = '*Deleted bookmarks*\n\n'
                    ixs_accepted = []
                    ixs_rejected = []
                    for ix in ixs:
                        try:
                            ix_as_int = int(ix)
                            if ix_as_int > 0:
                                ixs_accepted.append(ix_as_int)
                            else:
                                ixs_rejected.append(ix)
                        except:
                            ixs_rejected.append(ix)
                    ixs_accepted.sort(reverse=True)
                    for ix in ixs_accepted:
                        bookmark = bookmarks_db.get_rec_by_id(ix)
                        if bookmark:
                            message_body += Chat.format_message(bookmark, extended=True)
                            result = bookmarks_db.delete_rec(ix, delay_commit=True)
                            message_body += '\n\n'
                        else:
                            ixs_rejected.append(str(ix))
                    if ixs_rejected:
                        message_body += '*Deletion has failed for indexes:* {}'.format(', '.join(ixs_rejected))

I suppose .conn.commit() is not executed, hence the database remains open.

I will try .delete_resultset().

[LeXofLeviafan]

message_body += Chat.format_message(bookmark, extended=True)
result = bookmarks_db.delete_rec(ix, delay_commit=True)
message_body += '\n\n'

You don't seem to be using that result value anywhere… and there doesn't seem to be any benefit in waiting for the operation to complete before appending '\n\n' to message_body.

[sjehuda]

Done https://git.xmpp-it.net/sch/BukuBot/commit/3387dffeaaa06f8fc24ee968818d0982550a1553

---

Thank you for comment about result.

I have tried with .delete_resultset() as you have proposed and it appears to work well.

                case _ if message_lowercase.startswith('del '):
                    ixs = message_lowercase[4:].split(',')
                    ixs = set(ixs)
                    message_body = '*Deleted bookmarks*\n\n'
                    ixs_accepted = []
                    ixs_rejected = []
                    for ix in ixs:
                        try:
                            ix_as_int = int(ix)
                            if ix_as_int > 0:
                                ixs_accepted.append(ix_as_int)
                            else:
                                ixs_rejected.append(ix)
                        except:
                            ixs_rejected.append(ix)
                    ixs_accepted.sort(reverse=True)
                    ixs_approrved = []
                    for ix in ixs_accepted:
                        bookmark = bookmarks_db.get_rec_by_id(ix)
                        if bookmark:
                            ixs_approrved.append((ix,))
                        else:
                            ixs_rejected.append(str(ix))
                    if ixs_approrved:
                        for ix in ixs_approrved:
                            bookmark = bookmarks_db.get_rec_by_id(ix[0])
                            message_body += Chat.format_message(bookmark, extended=True) + '\n\n'
                        result = bookmarks_db.delete_resultset(list(ixs_approrved))
                    if ixs_rejected:
                        message_body += '*Deletion has failed for indexes:* {}'.format(', '.join(ixs_rejected))

Result:

Message

del 12,44,8,5,4,4a,ws,6800,10,2,3,6000,-5,0,2,6900,ws,10

BukuBot

Deleted bookmarks

6000. Untitled https://liliputing.com/rabbit-hopes-its-199-handheld-ai-device-succeeds-where-the-humane-699-ai-pin-fails/ None _dmp

6001. Untitled https://czar.kalli.st/life-hack/how-to-unlock-your-kst-page/ None _dmp

6002. 40 kuukautta Neuvosto-Venäjällä by Heikki Välisalmi | Project Gutenberg https://www.gutenberg.org/ebooks/73494 Free fb2 book and epub digitized and proofread by volunteers. _dmp

6003. My preferred agile heuristic https://calpaterson.com/agile.html What agile should be about and how to judge that _dmp

6004. Untitled https://actualactivists.com/2024/04/24/know-the-2024-dirty-dozen/ None _dmp

6005. Untitled https://linmob.net/mcf10-the-bug-and-the-bugfix/ None _dmp

6006. A cop pulls a gun on an unarmed teen. 7 years later, justice at last https://therealnews.com/a-cop-pulls-a-gun-on-an-unarmed-teen-7-years-later-justice-at-last A federal jury recently awarded $250,000 to Jawone Nicholson for pain and suffering after an off-duty Baltimore cop, Damond Durant, pulled a gun on him in Howard County. The award is the result of a protracted battle by the family to hold the officer accountable. _dmp

6007. 47 years later, Leonard Peltier is still not free https://therealnews.com/47-years-later-leonard-peltier-is-still-not-free Despite being eligible for parole since 1992, Peltier remains in prison—a sign of how determined the federal government is to repress and criminalize the American Indian Movement. _dmp

6008. Polls: More Americans Reject the 2 Party System, Sick of Trump and Biden By theconsciousresistance https://voluntarytube.com/videos/watch/7f08b3b5-7a2b-40f5-bec4-a91ba1cefb9b None _dmp

Deletion has failed for indexes: -5, 0, 4a, ws, 6900, 6800

Console:

Index 2 deleted
Index 3 deleted
Index 4 deleted
Index 5 deleted
Index 8 deleted
Index 10 deleted
Index 12 deleted
Index 44 deleted
Index 6000 deleted

Should not the delete process be from the latest (i.e the highest number) to the earliest (the smallest number) or is this not matter when using .delete_resultset()?

---

Done https://git.xmpp-it.net/sch/BukuBot/commit/3387dffeaaa06f8fc24ee968818d0982550a1553

[sjehuda]

Thank you for the help. I have forgot to mention you in the git message!

[LeXofLeviafan]

I think you misunderstood the API. What you're meant to pass into .delete_resultset() is fetched bookmarks data. (…Why are you fetching it twice anyway? :sweat_smile:) I.e. pretty much literally the output of .get_rec_all_by_ids() or any search methods.

Should not the delete process be from the latest (i.e the highest number) to the earliest (the smallest number) or is this not matter when using .delete_resultset()?

Aren't you feeding it data in the opposite order? The usage docs say it's expected to be pre-ordered.