microsoft / appcenter-sdk-android

Development repository for the App Center SDK for Android
Other
276 stars 134 forks source link

Fix max storage size limit #1649

Closed MikhailSuendukov closed 1 year ago

MikhailSuendukov commented 2 years ago

Description

When saving logs to the database, if the data size is too large to store them in the database, a separate file is created in which the contents of the log are stored. The problem is that when the database was full and the old record was deleted to create a new one, the separate log file associated with the deleted record was not deleted. Also, these files were not taken into account when calculating the amount of memory used. To solve the problem, the logic for deleting old records when the database is full was moved from the DatabaseManager class to the DatabasePersistence class. Also, a mechanism was created to clean up unnecessary data for previous versions.

Related PRs or issues

AB#94551

DmitriyKirakosyan commented 1 year ago

@MikhailSuendukov I've tested the changes and have 2 questions regarding the solution:

  1. Let's say the storage size is 10mb. And I have 10 files stored each of ~2mb in size. Then I try to store some big file, like ~11mb. It will be saved and then we will try to clean up the files because we exceed the size limit. Basically It will remove all the logs, because it will start from oldest and eventually will remove the newly added as well. Is it expected behaviour? Would it worth handing such cases to prevent unnecessary removal of small logs if we try to put a very big log, which would not stored anyway.
  2. I noticed that files from groupErrors are not removed while cleaning process. If it is expected, than this solution will not fix the issue described here https://github.com/microsoft/appcenter-sdk-android/issues/1646. I think we should remove error logs as well, if removal of other logs didn't help. What do you think? cc @MatkovIvan
MatkovIvan commented 1 year ago
  1. We'll not remove any. There is a check for it (not changed in this PR).
  2. Logs with "CRITICAL" flag must not be removed