[bug] view incrementation not working in production

[AlfonsoGhislieri]

Describe the bug Views are at times not incrementing correctly, it appears that at times when a user views a howto or research page the value on the firebase database is being incremented completely incorrectly. It seems like completely wrong values are being read from the database. I think it may even be the case that every user is getting different values?

I assume the issue is caused by something in the increment backend function here, but I have no idea what could be causing users to be getting different values from the database for the same ID.

To Reproduce Hard to reproduce as this does not appear to be an issue on the dev environment, but a bug which takes place when different users view a page in the production environment.

Expected behaviour Expect the views to increment by 1 from the value stored in the database whenever any user views a page. The value on the database should not be dramatically changing when different users visit the pages.

Screenshots This is something I saw earlier, the views on the howto page were 28 when I viewed it, when someone else viewed it it went down to 7. When I refreshed the howto page again it set the value on the database back to 28.

[thisislawatts]

@AlfonsoGhislieri can you include some more precise steps to recreate? I haven't seen the behaviour you've described when taking the following steps.

1. Logged into Browser A with user that has beta-tester access
2. Loaded individual How to: https://community.preciousplastic.com/how-to/nps_air-press-injector-v1
3. Observed that View Count was 14
4. Logged into Browser B with same user from step 1 that has beta-tester acces
5. Loaded individual How to: https://community.preciousplastic.com/how-to/nps_air-press-injector-v1
6. Observed that View Count was 15

[AlfonsoGhislieri]

Thanks for helping out @thisislawatts, sure here is what I've used to figure it out so far:

1. Check firebase stats for for howto - observed total_downloads: 1506, total_views: 50
2. Load inidivual howto https://community.preciousplastic.com/how-to/shredder-21
3. Observed that firebase stats now show total_downloads: 1500, total_views: 38
4. Observed that stats on page also show total_downloads: 1500, total_views: 38

Similarly for https://community.preciousplastic.com/how-to/nps_air-press-injector-v1 I think the views on your end are not the same as what I see on my end - at the time of writing this I have 25 views.

1. Check firebase stats

2. Observe that the views are the same when I load the page on my computer

3. Log into website on my phone and visit page

4. Observe different views on page and firebase

The actual incrementation works, but it the value that is being incremented is not always the same. The database is giving back different values for users on different devices maybe? It's like everything is out of sync. The only thing I could think is causing this issue is caching? But I would have assumed the cache data should be the same for every user?

[AlfonsoGhislieri]

Just to further highlight the issue, the shredder howto which earlier I observered having: total_downloads: 1506, total_views: 50 and then total_downloads: 1500, total_views: 38, now has total_downloads: 1497, total_views: 6

[chrismclarke]

One thing to take into consideration might be that all users store a cache of each howto in a local indexeddb, and that is only updated if the _modified field changes to avoid repeatedly retrieving the same information.

Howtos were originally designed as something that would be reasonably static and limited in number, so the approach has been to sync all howtos to the local cache on load and serve in an offline-first way. Having total view and download counters might not quite work well under these conditions, so before diving in any deeper I'd say to check whether the issue is a caching one or something else.

So I expect the issue might be if the view counts are calculated client-side then each user updates from their own cached counter, one user could push 6->7, and the next 20->21, then the first again 7->8

Easiest way to check the cache is the indexeedb browser, and to clear the cache either use an incognito tab or clear from the developer console. You could also try using one multiple browsers to test this theory.

Assuming this is the case, a couple options might be:

Short term - force the user to retrieve the howto from the server before updating count, the doc.get method should have an override param like doc.get('server') I think.

Medium term - instead of updating locally add a firebase function that updates server side. You can provide a utility value of Firestore.Increment which will just update the target field by a given number

Long term - move the dynamic parts of the howto into a separate/sub collection which is never cached, and use server-side functions to manage update and/or aggregation

[thisislawatts]

Thanks for the explainer here @chrismclarke, very helpful 🥇

So it looks like we want to remove the keep_modified_timestamp from this dbRef.set method?

await dbRef.set(
  {
    ...updatedHowto,
  },
  { keep_modified_timestamp: true },
)

https://github.com/ONEARMY/community-platform/blob/master/src/stores/Howto/howto.store.tsx#L214:L214

[thisislawatts]

I agree that longer term we should split the download and view counters off of the howto document. It seems odd for these actions which can be taken by non-authors to result in modifications on the howto document.

[chrismclarke]

Thanks for the explainer here @chrismclarke, very helpful 🥇

So it looks like we want to remove the keep_modified_timestamp from this dbRef.set method?

I would say you will want to remove the timestamp if you need any of the updated information available on the main list page. Additionally there might still be an issue if a user with a cached howto navigates direct and calls the counter update before the db has had a chance to pull latest changes.

So a better solution would probably be to just force the howto to update from the server whenever the page is loaded (some sort of useEffect hook to run the db.get('server') and update the activeHowto, and only update any counters after this is done

[AlfonsoGhislieri]

Thanks @chrismclarke I think that's most likely the issue, just need to double check to make sure.

@thisislawatts I had added keep_modified_timestamp because everytime a view or download was incremented it would change the _modified field which was an issue (it would incorrectly show that the page had been edited).

I also think that db.get('server')should solve the caching issue if we want to have a quick and easy fix. I would also be happy to start working on a firebase function that will just update server side (and then we can remove keep_modified_timestamp) like Chris suggested. I had chosen the local approach as it seemed quicker to implement, but didn't really think about caching at all, my bad..

In any case let me know!

[chrismclarke]

Thanks @AlfonsoGhislieri , all sounds good to me. Yeah caching is a pain, a lot of it comes from us trying to reduce the number of db requests (because firestore bills linearly with number of documents retrieved), but comes at a cost of not being as intuitive for developers.

We've had a lot of talk around trying to migrate to an SSR approach. I feel like the pattern of fetching fresh howto on page load and triggering updates backend lends itself nicely towards a future migration, whilst not adding many extra db requests (when compared to list-page updates) - so win-win.

Let me know if you have any challenges implementing either this or any future backend function (would probably say easier to keep as 2 separate PRs).

[davehakkens]

Thanks all for looking into it. Indeed the caching can be a pain. Usually its only in the details for developers, but feels like recently its slipped more into a few fundamental functional things for users.

So here a little side track on this caching issue. Recently we got more users saying they see complete how-to's wrong. Its starts mixing steps from how-to's, made an image below to illustrate the issue with a how-to. You can try yourself going from one how-to to the other. it tends to remember certain steps and show them in the next one.

Not sure if it helps to this dicussion. Just throwing the caching issues on the table

[chrismclarke]

Thanks @davehakkens - super weird bug, haven't seen it myself but good to know. I expect the fix planned for this issue should solve that one also, but can raise as a separate issue if it persists after.

[thisislawatts]

Are there any cost considerations around using db.get('server') we should consider before going down that path? As each view count would then cost a Read and Write operation? I don't have enough visibility into current usage on the underlying production Firebase project to know how this would affect billing.

It looks like the free usage per day is:

• Reads 50k
• Writes 20k

https://cloud.google.com/firestore/pricing

[AlfonsoGhislieri]

I spent a little bit of time testing it and all the incrementation issues are related to caching, fetching from the server did indeed fix it as @chrismclarke suggested.

In terms of cost @thisislawatts it looks like peak daily reads for the database in February was 1.1 Million, most days it's around 800k. Not too sure how much of an impact this many more reads will have on the overall cost.

I added a PR as a short term fix while I work on adding server side incrementation - removing the unecessary read every time a page is viewed. Option B, if costs are too high we could temporarily remove the views while the serverside functionality is added in?

@davehakkens I also just observed this bug, does not seem to be related, seems an issue of state.

[thisislawatts]

@davehakkens for visibility I split your comment here around the incorrect howto steps loading into a separate issue. #2123

[davehakkens]

Not sure if this serves. But as background information usually the costs are +/- €15 Month. And this is where they go. No idea what those extra writes will mean. But always up for trying to find out (:

[onearmy-bot]

:tada: This issue has been resolved in version 1.39.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: