wekan / charts

Wekan Helm Charts to deploy to Kubernetes
MIT License
3 stars 15 forks source link

feat: add in-memory cache folder for /data #15

Closed Nightreaver closed 1 year ago

Nightreaver commented 1 year ago

as discussed here https://github.com/wekan/charts/issues/14 this is my PR to solve that issue.

I'm using a "in memory" emptyDir, because the folder doesn't contain persistent data, right? So its created together with the pod, taking some memory and its getting deleted when the pod is deleted. The data itself resides in the MongoDB database in the end!?

xet7 commented 1 year ago

@Nightreaver

Folder does contain persistent data. Moving file from MongoDB GridFS to filesystem permanently copies file to WRITABLE_PATH and deletes it from MongoDB GridFS. Also, if files are stored in RAM, and there is more files that available RAM, that is another way to get data loss.

@mfilser @imajus @NotTheEvilOne What do you think?

xet7 commented 1 year ago

Related attachments and avatars migration code is at:

Nightreaver commented 1 year ago

Here ( https://github.com/wekan/wekan/pull/4336#issuecomment-1039231093 ) is written that [...] It is used only to store the files temporarily. The persistent files storage is still GridFS In this case my solution should work.

If however it has changed in the meantime, to a permanent place to store data, I need to use a proper persistent volume.

But overall "move attachments between MongoDB GridFS and filesystem" and " file is validated before moving to final storage" are not really clearing up what is actually happening.

xet7 commented 1 year ago

There are 2 different ways WRITABLE_PATH is used:

a) Migrations move attachments from MongoDB CollectionFS => Filesystem temporarily => MongoDB Meteor-Files

b) Multi-file storage. There is button at Admin Panel to move all attachments from MongoDB to Filesystem and back. Also each attachment at board has option to move attachment from MongoDB to filesystem and back. https://github.com/wekan/wekan/pull/4484

@mfilser Please correct if I described anything incorrectly. Thanks!

NotTheEvilOne commented 1 year ago

Hi,

I would recommend to disable the feature by default. /data/attachments may contain the only valid copy of an uploaded file as long as the file is being validated by e.g. an external program. Data loss may occur in this case as previously said.

Furthermore if I remember correctly avatars are not stored on GridFS. Setting the whole /data folder as in-memory would effectively erase all avatars on pod redeployment.

xet7 commented 1 year ago

Fixed at https://github.com/wekan/charts/pull/16

mfilser commented 1 year ago

@xet7 Correct. WRITABLE_PATH should be always at a permanent storage. If not, Multi-file storage get's corrupt.