On Wed, Aug 5, 2020 at 9:42 AM Ashutosh Sharma <ashu.coe...@gmail.com> wrote: > Yeah, it's being tested on the main table, not on a toast table. I've > removed this test-case and also restricted direct access to the toast > table using heap_force_kill/freeze functions. I think we shouldn't be > using these functions to do any changes in the toast table. We will > only use these functions with the main table and let VACUUM remove the > corresponding data chunks (pointed by the tuple that got removed from > the main table).
I agree with removing the test case, but I disagree with restricting this from being used on the TOAST table. These are tools for experts, who may use them as they see fit. It's unlikely that it would be useful to use this on a TOAST table, I think, but not impossible. > Another option would be to identify all the data chunks corresponding > to the tuple (ctid) being killed from the main table and remove them > one by one. We will only do this if the tuple from the main table that > has been marked as killed has an external storage. We will have to add > a bunch of code for this otherwise we can let VACUUM do this for us. > Let me know your thoughts on this. I don't think VACUUM will do anything for us automatically -- it isn't going to know that we force-killed the tuple in the main table. Normally, a tuple delete would have to set xmax on the TOAST tuples and then VACUUM would do its thing, but in this case that won't happen. So if you force-kill a tuple in the main table you would end up with a space leak in the TOAST table. The problem here is that one reason you might force-killing a tuple in the main table is because it's full of garbage. If so, trying to decode the tuple so that you can find the TOAST pointers might crash or error out, or maybe that part will work but then you'll error out trying to look up the corresponding TOAST tuples, either because the values are not valid or because the TOAST table itself is generally hosed in some way. So I think it is probably best if we keep this tool as simple as possible, with as few dependencies as we can, and document the possible negative outcomes of using it. It's not impossible to recover from a space-leak like this; you can always move the data into a new table with CTAS and then drop the old one. Not sure whether CLUSTER or VACUUM FULL would also be sufficient. Separately, we might want to add a TOAST-checker to amcheck, or enhance the heap-checker Mark is working on, and one of the things it could do is check for TOAST entries to which nothing points. Then if you force-kill tuples in the main table you could also use that tool to look for things in the TOAST table that ought to also be force-killed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
