Skip to content
This repository has been archived by the owner on Aug 19, 2018. It is now read-only.

Purging a folder does nothing. #23

Closed
kf6kjg opened this issue Oct 1, 2015 · 11 comments
Closed

Purging a folder does nothing. #23

kf6kjg opened this issue Oct 1, 2015 · 11 comments

Comments

@kf6kjg
Copy link
Contributor

kf6kjg commented Oct 1, 2015

And I finally come around to why I wanted to be able to compile and run this on my Linux machine - not having a working Windows install I needed to run Halcyon under Linux to debug a problem we've been having on our grid for quite some time now. This issue report is about that bug - now that I've conclusively found the root of the problem.

The user's view of the problem: Purging an inventory folder from the Trash causes the folder to disappear, but it shows back up in the Trash after a relog.

The developer's view of the problem: The line that does the work has been commented out with "will fix in the next patch" - see https://github.com/InWorldz/halcyon/blame/master/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs#L1504
It's high time for that patch from whomever commented that line out! :P

On an aside: this is the bug that Vin thought was because we are running on MySQL instead of Cassandra. However the smoking gun indicates that this will happen on all grids no matter the storage implementation. BTW: I complement whomever designed the migration process. Also whomever designed the abstraction layers: while it was a LOT of work to follow the logic through all the indirection and polymorphism, it's a nicely extensible system.

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 1, 2015

Because it bugged me, I've been working a fix. Almost there, but my analysis showed that while I solved the ability to purge a specified folder from the trash, I still possibly problem in that emptying the trash could leave dangling items in the database: it currently isn't doing a recursive delete, only a first-level.

Ran out of time this morning, will make further progress tonight or tomorrow unless I hear that this issue has already been solved! :D

@ddaeschler
Copy link
Contributor

The packet handler in your first comment is truly not used, and Cassandra inventory is able to purge. There was an erroneous protection for purge added to mysql towards the end and we never fixed it because we migrated.

You've probably figured this out by now, but just thought i'd chime in.

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 2, 2015

I beg to differ on the point that this function isn't called: that's how I found it; I placed a breakpoint in InventoryItemBase CachedUserInfo::FindItem(UUID itemId) as my scattering of debug statements were showing that the CassandraMigrationProviderSelector was indeed selecting the LagacyMysqlInventoryStorage, &c, and while this operation was selecting the checked inventory API, none of the folder purge functions in CheckedInventoryStorage were being called.

So in shotgun scattering more debug statements, breakpoints not working on the deep side of polymorphic calls for some reason, I found that FindItem was being called. Setting a breakpoint there worked. I then jumped back up the stack trace and found myself at line 1496 - in RemoveInventoryFolder(). And then I read the commented code.

Just before I posted the second message above I'd been having to build the several layers of middleware code that connects RemoveInventoryFolder() through to the selected underlying storage implementation. I've gotten Trash'd folders to now be able to be purged, but as I mentioned my analysis is showing that the current process I've implemented isn't recursive, so I need to verify that I'm using the correct methods and not leaving garbage behind.

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 2, 2015

What's more I just replicated the bug on the live InWorldz grid.

Here's how to test:

  1. Create a new folder in your inventory. It will be named "New Folder".
  2. Delete the new folder.
  3. Observe that the "delete" has moved the folder to the Trash, and the "New Folder" is sitting right at the top. This is correct and expected.
  4. Right-click on the "New Folder" sitting in the Trash and select "Purge Item".
  5. Observe that the folder disappears from your inventory. This is correct and expected: the client removes the folder from its own view of inventory.
  6. Relog.
  7. Open Inventory and expand your Trash folder.
  8. Observe to see if the "New Folder" is in the Trash.

If step 8 shows the "New Folder", then the bug is still in effect.

Replicated with the following setup:

Firestorm 4.7.3 (47323) Aug 17 2015 18:14:04 (Firestorm-Releasex64) with OpenSimulator support

You are at 145.0, 138.0, 19.8 in Nargo located at vm-1-4.inworldz.com (198.61.243.4:9035)
SLURL: http://places.inworldz.com/Nargo/145/138/20
(global coordinates 250769.0, 255370.0, 19.8)
InWorldz 0.9.17 R5720

Side note: I'm not sure why I'm in this region, it's just where I happened to be when I logged in. If you have a better place for me to do testing, please let me know!

@ddaeschler
Copy link
Contributor

Purging a single item and purging the contents of an entire folder are two different operations and two different bugs in this case. However, considering that people can change outfits, I know that single links can be purged, that's why I'm scrutinizing and questioning exactly what is happening in your tests. I'll need to replicate this.

From what I remember I thought the empty trash bug was an overzealous security test for mysql. You can indeed empty your trash on cassandra. So you may have found a different bug.

@ddaeschler
Copy link
Contributor

Oh wait.. Maybe im misunderstanding this bug.

I thought the problem on mysql was that you couldn't empty the trash. Is this simply about removing single folders out of the trash itself?

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 2, 2015

Right now it's both. At the start I'd not realized how separated the codepaths are. Live grid: can empty trash, cannot purge individual folder; Halcyon on MySQL: cannot do either.

However I reiterate that the code paths I've followed don't get to storage-implementation-specific stuff until much later - assuming I've been following them right. It's been hard to keep a map in my head while learning it.

I also think I found a third, more insidious, bug. This one would be not user-facing: it would simply result in disconnected inventory. You might try running the Cassandra equivalent of the following SQL against your database to make sure you've not got disconnected inventory:

/* Verify that all items in inventory have parent folders. */
SELECT * FROM inventoryitems invi WHERE invi.parentFolderID not in (SELECT folderID FROM inventoryfolders);

/* Verify that all folders in inventory have parent folders - excepting the root folders of course. */
SELECT * FROM inventoryfolders invf WHERE invf.parentFolderID not in (SELECT folderID FROM inventoryfolders) AND invf.parentFolderID != "00000000-0000-0000-0000-000000000000";

Both queries shouldn't return any rows. If they do, the results are the items that should have been purged but were not.

I've run out of time again, but I think in the morning I'll trace and verify this last bug in a new branch where I can see things as they stand: if it does exist it's a worse problem as it's hidden from normal view.

@ddaeschler
Copy link
Contributor

There are protections in the cassandra code against assigning null parent IDs. The same probably isnt true for the MySQL code.

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 2, 2015

Ah, but this wouldn't be null parent IDs: it would be that the folder was deleted but the items and folders in it were not. Hence the parenFolderID would be pointing to a folder that no longer exists.

@ddaeschler
Copy link
Contributor

In Cassandra storage an item cannot exist without its folder. The folder is actually something called a supercolumn and the items are stored along with it

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 2, 2015

I've forked this issue into its constituent parts to hopefully alleviate some of the confusion and conflation.

@kf6kjg kf6kjg closed this as completed Oct 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants