search

ARK-Builders / ARK-Navigator

Android app for navigation through your data
MIT License
15 stars 15 forks source link

Stats storages and crash in Aggregate mode #421

Open tuancoltech opened 7 months ago

tuancoltech commented 7 months ago

:rocket: Summary

Fix crash in Android Navigator, Aggregation mode. Issue: https://github.com/ARK-Builders/ARK-Navigator/issues/412

kirillt commented 6 months ago

https://github.com/ARK-Builders/arklib-android/pull/125 merged

kirillt commented 6 months ago

@tuancoltech am I right that we only need to bump arklib-android version before merging?

tuancoltech commented 6 months ago

@tuancoltech am I right that we only need to bump arklib-android version before merging?

@kirillt This PR doesn't depend on any change in arklib-android, so please feel free to merge it :)

kirillt commented 6 months ago

@tuancoltech there are some unresolved references in the last build.

sonarcloud[bot] commented 6 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

kirillt commented 6 months ago

Another concern is that the fix is applied to TagLabeledNStorage, but there are also 3 other similar storages: TagLabeledTSStorage.kt, TagQueriedTSStorage, TagQueriedNStorage. These storages have the same bug, they should be fixed, too.

kirillt commented 6 months ago

But the best approach would be to re-design stats storages... They were implemented during autumn 2022. At summer 2023, we've refactored storages completely and separated reusable primitives. See FileStorage and FolderStorage from arklib-android.

In fact, the data stored in stats storages is very simple key-value mapping. I think, it's possible to implement them similarly to ScoreStorage. Scores storage is id-to-integer mapping, and stats storages are either tag-to-integer or tag-to-timestamp. But in any case, the code will be very similar to scores storage.

mdrlzy commented 6 months ago

I think, it's possible to implement them similarly to ScoreStorage

They are not quite like ScoreStorage. They can often receive new events (once per second), which will provoke overwriting of large file (there is only one for the entire root). Here we need to implement a new storage, which will have a debounce flush to disk for new entries.

I think there is no point in fixing them here; it’s better to move and refactor in arklib-android.

kirillt commented 6 months ago

@mdrlzy good note about debouncing. We can implement such a parameter in our base storage classes. Then stats storage should be similar to score storage, just debounce parameter would differ.

kirillt commented 5 months ago

@tuancoltech let me know if you need help with implementing StatsStorage

tuancoltech commented 5 months ago

@tuancoltech let me know if you need help with implementing StatsStorage

@kirillt Sure, I'll ask here in the case.

kirillt commented 5 months ago

@tuancoltech We might need small refactoring in arklib-android because BaseStorage hard-codes ResourceId as the type acceptable as keys of a storage. In case of tag stats, keys belong to the String type.