[Support] BukuBot issue with database lock error about buku HOT 6 CLOSED

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 😅

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 😅)
6. You should probably either delete the val = line, or use the assigned value at least once.

from buku.

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().

from buku.

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.

from buku.

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

from buku.

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

from buku.

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? 😅)
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.

from buku.