Skip to content
This repository has been archived by the owner on Nov 25, 2022. It is now read-only.

Limit retained history #244

Open
r10s opened this issue Aug 20, 2018 · 36 comments
Open

Limit retained history #244

r10s opened this issue Aug 20, 2018 · 36 comments

Comments

@r10s
Copy link
Member

r10s commented Aug 20, 2018

to avoid taking up all device "disk"-memory, there should be an option to limit the retained history.

  • a simple, initial approach would be just to set a max. number of messages allowed in a chat; if more messages arrive, old are deleted (from the device, not from the server)
  • EDIT: the "contact requests are also a "chat" in this sense
    compared to using a fixed time, this would have the advantage that rarely used chats are not affected.
  • "starred" messages are not deleted
@testbird
Copy link
Contributor

testbird commented Aug 20, 2018

See https://support.delta.chat/t/reduce-space-on-internal-storage-disk-full/132/24
for a complete solution overview, that can also coordinate any server/local storage requirements, archiving, and syncing use-cases, while only needing 3 user-facing configuration items to handle all that.

@csb0730
Copy link
Contributor

csb0730 commented Aug 20, 2018

In combination with "vacuum" perfect shot in a first glance :)

... and really important meanwhile.

The local device should be in focus, really!

@csb0730
Copy link
Contributor

csb0730 commented Aug 20, 2018

Related to that, maybe it's worth to show some simple statistics:

  • in a chat's profile (or somwhere) how many messages belonging to that chat
  • in general: size of current database
  • in general: free memory (important to do a vacuum of database)

And finally: what about different location (external sd card) for attachments or database?

@r10s
Copy link
Member Author

r10s commented Aug 21, 2018

And finally: what about different location (external sd card) for attachments or database?

this would degrade security and/or make a secure system more complicated, so, at least for now, i would not vote for this.

@csb0730
Copy link
Contributor

csb0730 commented Aug 21, 2018

Ok understood. I suspected that this comment will arrive ;-)

Then I think the issue "different location" (You mean only that?) should excluded here and I will open a separate new issue for that.

This is the better approach :)

@r10s
Copy link
Member Author

r10s commented Aug 21, 2018

yes, "different location" and sd-card is an different issue with its own problems :)
from a security point of view, it should not be supported - esp. as newer android versions allow to use the sd-cards to upgrade the memory in a transparent way so that things stay encrypted.

@csb0730
Copy link
Contributor

csb0730 commented Aug 23, 2018

Hi @r10s
I see this issue as one of the important ones for now. How can I support? Please give me some hints and I'll investigate into!

There are some prerequisites before:

  • show some statistics for user to visualize the individual situation
  • add options to chats to limit retained history
  • add functions to add "stars" to messages
  • implement vacuum function for database

@r10s
Copy link
Member Author

r10s commented Aug 23, 2018

yeah, it is important, but there are many important ones curently :)

from the core, some prerequisites are already fulfilled, eg. starring messages is already implemented in the core.
what we're currently doing is to change the ui from telegram-ui to signal-ui, once done, so i would not add option on this in the old ui.

@testbird
Copy link
Contributor

testbird commented Aug 25, 2018

Just noticed that limiting the chat history won't help either, because even a device with very few chat messages has an ever growing database.

I think this is because all the incoming email messages (aka "contact requests") are never automatically deleted, i.e. when they get removed from the server's INBOX folder, usually by other MUAs. (Having to delete classic emails in deltachat is kind of a dangerous and unnecessary task.)

Reopen?: "Emails deleted/moved from INBOX on server, need also be removed from "contact requests"" #195 (It's basic IMAP server usage.)

@csb0730 your optional configuration wishes deltachat/deltachat-android#321 and deltachat/deltachat-android#327 remain untouched

@r10s
Copy link
Member Author

r10s commented Aug 25, 2018

I think this is because all the incoming email messages (aka "contact requests") are never automatically deleted

currently, this is true.

however, the deletion of old messages we're taking about here include the contact requests (which are also only messages).

@testbird
Copy link
Contributor

testbird commented Aug 25, 2018

Hopefully you just haven't thought it through yet before answering, deleting the x+1st email from the INBOX sounds kind of wrong.

I mean when there was work done on an outline like #120 + #195 it doesn't mean the checkmarks can't be further improved, or implemented in selective steps, like first only implementing auto-deletion as you seem to mean with this issue.

@r10s
Copy link
Member Author

r10s commented Aug 26, 2018

Hopefully you just haven't thought it through yet before answering,

well, this happens all the time to me :)

deleting the x+1st email from the INBOX sounds kind of wrong.

however, in this case i think it is fine. the issue is about deleting messages from the device, not from the server.

@testbird
Copy link
Contributor

testbird commented Aug 27, 2018

Hopefully you just haven't thought it through yet before answering,

well, this happens all the time to me :)

Ha, ha, yes :-) I could only guess, because... you know, happens to me too all the time, and that's how we improve things :)

I consider the incoming emails (old "contact requests") to be under the "authority" of classic MUAs, while the chat folder is under the "authority of deltachat. And the IMAP server is to be used as "authoritative" to sync between clients. But some clients may optionionally keep larger archives.

By default, the INBOX gets sorted out by the user using or configuring the classic MUAs, but deltachat can allow the user to start a chat by replying and directly delete junk from the INBOX.

Now, if deltachat would delete incoming email only locally, that would not allow providing the user with a consistent view between MUAs. It would be a change from the current deleting behaviour, and it would later have to be changed again (to also delete on server) to allow that multiple clients provide a synchronized view.

  • Deltachat must autodelete chat messages by default locally AND in the servers's chat folder, to limit server space usage (otherwise, no consistent way for the user to delete to prevent filling up the server) and to provide multi-client consistency.

  • [EDITED into a separate point] Deltachat may optionally support to keep local message archives of different sizes.

  • Deltachat should not auto-delete emails from the servers' INBOX by default, as that is in the authority of classic MUAs. And not auto-delete emails from the local "incoming emails" list, to stay consistent with the view in other classic MUAs.

I don't think the general simplification to only-delete-locally would allow a complete solution to the problems.

With others working on the frontends, I hope you'll be able to do the little refinements to the core that bring the email-chat compatibility concepts to perfection.

@r10s
Copy link
Member Author

r10s commented Aug 27, 2018

Deltachat must autodelete by default locally AND in the chat folder, to limit server space usage and provide muli-client consistency. With optionally keeping a larger local history.

Deltachat must never auto-delete by default locally OR in the INBOX, to provide a consistent view accross MUAs.

i do not think this is true. EDIT: i've overseen the "by default", however, i am still not sure such an option is needed and that not deleting most messages is a good default to safe memory on the device.

imagine a multi-device setup with an android phone with few memory and a desktop mua. while the android phone may have 512 MB available for the database and lets deltachat delete messages from the device - why should these messages not stay on the desktop mua?

also, not having the whole messages archive on a device i carry with me may be wise from a security point of view - but this does not mean the messages cannot stay on another device at home.

@testbird
Copy link
Contributor

testbird commented Aug 27, 2018

Yes, that was exactly the reason for the "archived" property for messages in #120. It provides independence between clients and the server. For example, on a small phone the archive can be cut down (or even disabled) differently as the archive on the desktop. And at the same time, the space used on the server is also independent from all the clients' archive sizes.

The idea is for the active chats to get synced, but the archives to be device specific.

If a chat message gets deleted on the server, the clients locally deletes active chat messages, but keeps the message if it has the "archived" state (only local archive purging rules apply).

If a chat message is deleted on a client and there is no "archived" message property implemented, the user would need to be able (and have to manually decide, with an annoying question) whether to only delete locally or also on the server (to not have to remove it again from all other clients).

EDIT: With the "archived" flag available, deletions of messages can always be carried out on on the server as well (if still there). The desktop client may archive all messages right away, and the user effectively only working with archives then, to ensure a local copy is always kept on the desktop.

(Hope I could explain that in an understandable manner.)

@testbird
Copy link
Contributor

testbird commented Aug 27, 2018

i am still not sure such an option is needed and that not deleting most messages is a good default to safe memory on the device.

EDIT: I'm not sure what you meant here. Didn't I always mean to delete on server, too?
What are you refering to?

Do you mean me not wanting to auto-delete emails from the local "incoming emails"?

  • I can see a case for a local-only limit with users that may not even have any subfolders but large (multi GB) INBOXes and never manually delete or clean up their email INBOX. The long list of incoming emails may not bother the user, but can be too much for the mobile data or the device. So an optional "show only the last 100 emails" (and locally purging any further messages) could make sense as a measure against message overflow, but it could interfere with email compatibility and maybe later showing subfolders in the incoming emails (contact requests).

@testbird
Copy link
Contributor

testbird commented Aug 27, 2018

But the basic problem is of course always that deleting only locally does not prevent running out-of-space used on the server, and creates the annoyance to have to manually repeat every delete on every device. (Therefore I see the "show only the last 100 emails" as an exception, just to protect from large INBOXes.)

@testbird
Copy link
Contributor

If you mean this issue as fixing things for a suitable v1.0 -core, it may be seen as equivalent to implementing "Chat message selection", "Deletion management" without the bulk delete UI, and vacuum from "Storage Restrictions" in #120, while its "Archive management" could stay for the later multi-client support.

@r10s
Copy link
Member Author

r10s commented Aug 27, 2018

But the basic problem is of course always that deleting only locally does not prevent running out-of-space used on the server

true, but i would regard this as different issues.
also, at least on the devices from users around here, deleting from devices is much more urgent than deleting from the server (also as the latter can be done by other apps).

wrt VACUUM: this is no no-brainer: during execution of the statement the database may be take up double place. also this is a typically long-running command, so it might not make much sense to call it automatically.

@csb0730
Copy link
Contributor

csb0730 commented Aug 27, 2018

Agree, vacuum is important and space consuming (at least double size of db) and should be executed manually. I tried it manually with a backup db at desktop. Maybe it's necessary to lock database while execution. I think depending from the device it can take many minutes!

Agree too, deleting from device is urgent because at current state I really suspect database is growing endless and no chance to shrink it at device. Sooner or later space runs out and then even a backup is impossible :-/

For me the local device is more important to attack because here I have to live with dc's possibilities, at server I have multiple possibilities to deal with :-)

@r10s
Copy link
Member Author

r10s commented Aug 27, 2018

For me the local device is more important to attack because here I have to live with dc's possibilities, at server I have multiple possibilities to deal with :-)

also for me this is much more urgent :)

@testbird
Copy link
Contributor

Could make sense for DC to limit the DB size to half of the free storage size, to trigger vacuum, and by this allow receiving new messages at all times.

@testbird
Copy link
Contributor

testbird commented Aug 28, 2018

But the basic problem [of only deleting locally] is of course always that deleting only locally does not prevent running out-of-space used on the server

true, but i would regard [deleting on server] as different issues.

Different thing to solve, yes, the formality itself isn't too important for me, I rather think it is important to have the related things written down and public in an open issue, where it stays to document an area where solving only one part creates bugs/problem in other parts:

  • only deleting locally -> server chat folder never cleaned => also delete on server
  • deleting locally and on server (small local storage) -> sync problem (large local storage not used) => device independent archives
  • current "archived" chat state is rather a disable or hide state -> improve feature to "archived messages" so that archives are not revived completely with new messages coming in.

also, at least on the devices from users around here, deleting from devices is much more urgent

Seems we all agree on that. And we know the next problems that will occur, and can stay on top of things by keeping a properly edited issue for the related things open, until the area gets fixed completely.

@csb0730
Copy link
Contributor

csb0730 commented Sep 1, 2018

Last days I learned that main problem are attachments which are stored in database. I had mail conversation with @r10s regarding that issue.
Beside that I see big storage usage additionally to database size.

A secure finding is, that after deleting big attachments the vacuum function is absolutely required. No chance without using that to reduce storage space.

Unclear for me is in the moment why additional storage is required beside database storage space.
I think it should be documented what is stored where and why.

Some findings:

  • attachments are stored in database (only received?) or even sent?
  • do not know why many additional space is required beside database size !

(all this device local !)

@r10s
Copy link
Member Author

r10s commented Sep 1, 2018

@csb0730 normally, attachments are not stored in the database. instead they are stored as normal files beside the database.

when doing a backup, however, to have a single, handy file, these normal files are added to the table backup_blobs. on an import, these files are copied back to nomal files, the table is deleted and the database VACUUM'd.

this could be optimized by importing to a new database using ATTACH and INSERT INTO so that a final VACUUM is not needed and temporary less space is needed, however, before we optimize on this, imo things as #149 should be solved.

@csb0730
Copy link
Contributor

csb0730 commented Sep 2, 2018

... these files are copied back to nomal files, the table is deleted and the database VACUUM'd.

This seems to me a reasonable approach for now :)

But what worries me a little is the big difference between the size of backup file (1,05GB, which then should include all data) and the size of total memory reported by app manager for DC (1,77GB).
How to explain that?

@r10s
Copy link
Member Author

r10s commented Sep 2, 2018

How to explain that?

have you tried a re-install of delta and import the backup?
or, event better for reproducing, do the whole process with a "normal and untouched" (not manually modified db) installation?

@csb0730
Copy link
Contributor

csb0730 commented Sep 2, 2018

Hi @r10s,
Yes, with my last install (the self done apk) I was forced to export the backup and import again.
The import process tooks around 20 min but at the end all looked good, so far. I used an untouched backup from previous version (v0.19.0uuencode2). Did no modifications of backup file.
==> So with a successful import we should see a similar size for database size and reported memory size in app manager ?

(What I described above were some examinations with the backup db file at PC and sqlite behaviour around vacuum command.)

But because memory of my phone was short I suspect that maybe the vacuum command at the end of import failed? That could explain the picture now! Then the memory in database is not free'd and I have the files in blob dir AND the big database with unused space too?!

Is it possible to examine the original database at device? Maybe copying to PC by adb or copying it to download dir?
I can't achieve it because of missing rights with adb.

@csb0730
Copy link
Contributor

csb0730 commented Sep 2, 2018

Depending on outcome now: Maybe it's more reliable to use an extra db file for all attachments in backup because then the vacuum wouldn't be so memory critical at import (vacuum)! Extra db file can be deleted and that's it.
It gets interesting now ;-)

@csb0730
Copy link
Contributor

csb0730 commented Sep 3, 2018

Some test done with a fresh backup from dc v0.20.0 (db size is 1001.9 MB) at PC:

  1. Test: Launch sql statements exactly as listed in sources (dc_imex.c):

drop table backup_blobs;
vacuum;

Result is:

  • Journal file is created (865.5 MB, stays until changes are written to db)
  • Error message "cannot VACUUM from within a transaction: vacuum;"
  • "vacuum" is not performed after this error message.
  • Table "backup_blobs" is deleted
  • db size stays at full size (1001.9 MB)

"vacuum" cannot performed, even manually after db was closed and changes written to db!
sql command "end transaction" is necessary before it performs!

  1. Test: Launch sql statements with additional "end transaction" before "vacuum" command:

drop table backup_blobs;
end transaction; (<== no success with "commit" ! )
vacuum;

Result now:

  • Journal file is created (865 MB)
  • Journal file is deleted automatically
  • after some seconds next journal file is created (around 130 MB).
  • No error message
  • vacuum performed and db size shrank down to 137 MB.

Conclusion:
It seems that "end transaction" is important to perform between "drop table" and "vacuum" to get vacuum working!

@r10s: can You confirm this?

@csb0730
Copy link
Contributor

csb0730 commented Sep 3, 2018

I think the following code should be used in import_backup() near line 1091 in dc_imex.c:

dc_sqlite3_execute(context->sql, "DROP TABLE backup_blobs;");
dc_sqlite3_execute(context->sql, "END TRANSACTION;");
dc_sqlite3_execute(context->sql, "VACUUM;");

This should get the code working as expected (trial at real device outstanding !)

@csb0730
Copy link
Contributor

csb0730 commented Sep 5, 2018

I described many things regarding backup export import with last comments. The more I think about this the more I come to the conclusion that this is worth to open an extra issue for that.
It isn't really related closely to the issue described here!
==> Will open an extra issue for that to follow the problem described reladed backup and memory consumption.

Nevertheless will do some tests described in the last comments next days.

@csb0730
Copy link
Contributor

csb0730 commented Sep 5, 2018

Continuation of memory problem see new issue #261

@csb0730
Copy link
Contributor

csb0730 commented Feb 15, 2019

Comments until to 28 Aug 2018 are belonging to the real issue described !
Next comments belongs and lead to the backup issue #261!

@testbird
Copy link
Contributor

testbird commented Apr 11, 2019

I think, the best high-level solution overview, that came out of this issue and #120, is at:
https://support.delta.chat/t/reduce-space-on-internal-storage-disk-full/132/24

=> Solving all the different server and device storage requirement use-cases with only 3 simple user-facing items, and a default to always allow operation.

@csb0730
Copy link
Contributor

csb0730 commented May 7, 2019

Now we have the new UI and most of work relating that is done. But because of this isssue is a really necessary UX feature for daily use:

Is anyone working on that?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants