Add delete segment data update action

[frcroth]

URL of deployed dev instance (used for testing):

• https://deletesegment.webknossos.xyz

Steps to test:

• Delete segments in WK. You can ensure that they are deleted visually and via fossildb.

TODOs:

• Frontend: Add remove action to context menu

Issues:

• fixes Delete segment #7408

---

(Please delete unneeded items, merge only when none are left open)

• Updated changelog

[philippotto]

The front-end part should be done now. I noticed that the RAM allocation of the JVM went up quite a bit (and stayed there) during testing. Not sure whether this is just typical JVM behavior or if there is a memory leak somewhere?

Do we have any measures in place to avoid huge dataset modifications? Especially in combination with a fallback layer, this could quickly become a very long operation, I assume.

[fm3]

Hmm good point, this feature is not really prepared for fallback segmentation layers. In fact, currently, the backend will throw an error if this update action is used on a tracing without a segment index. This should definitely be checked in the frontend first.

When we add a segment index with fallback layer functionality, we might have to rethink this whole thing. Otherwise we would potentially update tens of thousands of buckets without meaning to…

@frcroth do you have any ideas about the memory consumption? My assumption would be that the jvm GC should take care of this once there is some memory pressure, but I haven’t looked into this in depth. If you know anything rightaway, let’s discuss it, but don’t spend too much effort researching this now unless we see actual problems. This is likely also related to our general numeric arrays problems (we will want to avoid allocating objects for each element in the future)

[frcroth]

@frcroth do you have any ideas about the memory consumption? My assumption would be that the jvm GC should take care of this once there is some memory pressure, but I haven’t looked into this in depth. If you know anything rightaway, let’s discuss it, but don’t spend too much effort researching this now unless we see actual problems. This is likely also related to our general numeric arrays problems (we will want to avoid allocating objects for each element in the future)

I also think that is the cause because the code uses the expensive UnsignedInteger objects.

[fm3]

Backend LGTM and works :) true, it’s slow, but I’d say that’s #3810

Request for the Frontend: this option should only be enabled in case the volume tracing has a segment index.

[philippotto]

Request for the Frontend: this option should only be enabled in case the volume tracing has a segment index.

What about fallback data? Should I forbid this case, too? Otherwise, once we have segment indices for fallback data, this feature will be automatically available (and probably lead to problems).

[fm3]

Hmm, yes, let’s forbid it there as well. We can revisit this as part of #7132 or later

[philippotto]

Hmm, yes, let’s forbid it there as well. We can revisit this as part of #7132 or later

Ok, done.

Let me know if you need something else from me!

[fm3]

[frcroth]

@philippotto Does this need frontend review? If so, can you please assign a reviewer?

[philippotto]

Ok, I requested a review now.

[MichaelBuessemeyer]

Thanks for this PR enabling to 🗑️ a segment.

The code looks good to me 👍. I saw the "problem" with having to hand the resolve callback of a Promise via the action to the saga, but could not come up with a better idea :/. So let's keep it that way.

Just an idea that popped into my mind: The deletion of a segment is like a 3d bucket fill with id 0. Would it be possible to implement this as a feature with the overwrite id as a parameter, so it might replace the current 3d bucket fill done by the worker? But, this is a premium feature, isn't it? And maybe we do not want our users to wait for every bucket fill?

Testing results:

• I'd add the name of the segment to the warning modal shown before deletion of the data.
• I am aware that the feature is meant to only delete the (voxel) data of the segment, but after the deletion, the segment in the list is empty, which feels weird to me: Having a segment without any data in the list. I would kinda expect the segment being deleted from the list (although the menu item clearly is named something like "delete data") -> feel free to argue 😄

@fm3 Could you please check whether the feature works by check the fossil-db like felix suggested (In case you think this is testing step is worth it)? I do not know how to do this.

[fm3]

the segment in the list is empty

I agree, this is not ideal. I’d say the frontend should always send both the deleteSegmentData and deleteSegment (from list) update action

Could you please check whether the feature works by check the fossil-db

Looking good 👍

[philippotto]

Just an idea that popped into my mind: The deletion of a segment is like a 3d bucket fill with id 0. Would it be possible to implement this as a feature with the overwrite id as a parameter, so it might replace the current 3d bucket fill done by the worker? But, this is a premium feature, isn't it? And maybe we do not want our users to wait for every bucket fill?

I think, this is a good idea, but nothing for this PR. We have to be careful about not putting too much workload on the server...

I'd add the name of the segment to the warning modal shown before deletion of the data.

Done.

the segment in the list is empty

I agree, this is not ideal. I’d say the frontend should always send both the deleteSegmentData and deleteSegment (from list) update action

I only agree partly. If the user segmented something incorrectly and they want to start from scratch, chances are that they already set a custom name and color for the segment. When automatically deleting the entry from the list, these properties are gone, too. I'd rather delete too little than too much. However, I also see where you are coming from. That's why I pushed a trade-off: After the deletion, a success toast is shown which allows to also delete the segment from the list with one click.

I would rather have used a checkbox in the confirm-modal, but this proved to be a bit too difficult.

I think, this trade-off is fine for a feature which is probably rather rarely used. I hope you agree 🤞

[philippotto]

I think, it's fine to merge. My last change was quite small and I just doublechecked it.