Plots2: Tag subscription counts wrong on stats page

[jywarren]

We just changed the query for this page and it looks like something went wrong, and perhaps the grouping is incorrect? There are so many tags with 219 followers and almost none w 1 follower.

https://publiclab.org/stats/subscriptions

Let's check the last change to that file and figure out what went wrong.

[dongskyler]

Why 219 subscribers?
Is 219 some special number in our code? Does it ring any bell?
Is the total number of subscribers 219?
Do you have any clues or guesses for debugging this issue?

[jywarren]

Hi @dongskyler! Sorry, i think we must've given you a mistake in our suggested code change! Thanks for looking into this! We SUPER appreciate your help, and please don't feel this was your fault in any way! 😄

Hmm. I wonder -- it's a pretty odd bug. The change was to add the .joins() statement on this line:

https://github.com/publiclab/plots2/blob/0e58e62edd5efe6c9f22fce6ac24bf770b932961/app/controllers/stats_controller.rb#L4

Is it possible we're joining incorrectly? What if we joined against something which has only 219 records... or something... ?

[jywarren]

TagSelection.count => 5006

So, it's not that. Could it be that... the returned records are not of the same type? So the logic in this block is not working as we had expected?

https://github.com/publiclab/plots2/blob/0e58e62edd5efe6c9f22fce6ac24bf770b932961/app/controllers/stats_controller.rb#L2-L9

Let me test what TagSelection.where(following: true).joins(:node_tags) is versus TagSelection.where(following: true)...

[jywarren]

irb(main):003:0>  TagSelection.where(following: true).joins(:node_tags).last
=> #<TagSelection user_id: 994, tid: 13841, following: true, created_at: "2000-01-01 00:00:00", updated_at: "2019-06-15 17:58:42">
irb(main):004:0>  TagSelection.where(following: true).last
=> #<TagSelection user_id: 694777, tid: 25416, following: true, created_at: "2020-05-19 15:40:23", updated_at: "2020-05-19 15:40:23">
irb(main):005:0>  TagSelection.where(following: true).joins(:node_tags).count
=> 15755
irb(main):006:0>  TagSelection.where(following: true).count
=> 4366

Hmm. So the filtering works well, but, why is the logic not working to actually show counts? Let me try going through the logic step by step in the rails console.

[jywarren]

OK, so what I think is happening is that the relation is defined here:

https://github.com/publiclab/plots2/blob/0e58e62edd5efe6c9f22fce6ac24bf770b932961/app/models/tag_selection.rb#L4

But, both tables have a tid column. So, it's actually fetching the node_tag with tid matching tag_selection.id? Or somehow that relation is wrong. I'm going to try manually writing the JOIN to test this theory: https://stackoverflow.com/questions/1509692/rails-activerecord-joins-with-left-join-instead-of-inner-join

[jywarren]

Aha!

I found that it's possible to use .group() to do this all in SQL!

TagSelection.where(following: true).joins("LEFT JOIN community_tags ON community_tags.tid = tag_selections.tid").joins("JOIN term_data ON term_data.tid = tag_selections.tid").group("term_data.name").count

Replaces the whole iterative loop, and is probably MUCH more efficient! No iterating over every tag... woohoo! I'm gonna replace this whole segment.

[jywarren]

Done in https://github.com/publiclab/plots2/pull/7930! Let's see how that does.

[jywarren]

Merged it! Let's test at https://stable.publiclab.org/stats/subscriptions as soon as it's done building.

[jywarren]

OK, it kind of worked. It's no longer showing 219s... but the new query may suffer from other issues. Re-opening!

[jywarren]

I think i shouldn't have done LEFT JOIN?

[jywarren]

That's right, i think both should be INNER JOIN but this still returns too many:

tags2 = TagSelection.where(following: true).joins("INNER JOIN community_tags ON community_tags.tid = tag_selections.tid").joins("INNER JOIN term_data ON term_data.tid = tag_selections.tid").group("term_data.name").count['water-quality']
=> 34087

https://github.com/publiclab/plots2/blob/c0de1f07aaee224a1a5c966a5985d47c22106d5a/app/controllers/stats_controller.rb#L3-L7

Here is the query being generated:

SELECT  `tag_selections`.* FROM `tag_selections` INNER JOIN term_data ON term_data.tid = tag_selections.tid INNER JOIN community_tags ON community_tags.tid = tag_selections.tid WHERE `tag_selections`.`tid` = 3838 AND `tag_selections`.`following` = TRUE LIMIT 11

[jywarren]

Ah, ok - so it's cross-maching and just returning a lot of duplicates. I can add a .distinct and this should be resolved i think?

[jywarren]

OK! So this works and returns proper counts:

    @tags = TagSelection.where(following: true)
      .joins("LEFT JOIN community_tags ON community_tags.tid = tag_selections.tid")
      .joins(:tag)
      .group("term_data.name")
      .count

However, it does not filter out spam. 😱

[jywarren]

We could TRY something like this:

TagSelection.where(following: true)
  .joins("LEFT JOIN community_tags ON community_tags.tid = tag_selections.tid")
  .joins(:tag)
  .joins("INNER JOIN node ON node.nid = community_tags.nid")
  .where("node.status = 1")
  .group("term_data.name")
  .count

However, it's a pretty slow query: maybe 3 seconds? We might be able to select fewer things to speed it up. But I dunno.

We could cache it every 24 hours?

[jywarren]

Caching would look like:

Rails.cache.fetch("stats/subscriptions/query", expires_in: 24.hours) do
  TagSelection.where(following: true)
    .joins("LEFT JOIN community_tags ON community_tags.tid = tag_selections.tid")
    .joins(:tag)
    .joins("INNER JOIN node ON node.nid = community_tags.nid")
    .where("node.status = 1")
    .group("term_data.name")
    .count
end

[dongskyler]

That sounds like a rather inefficient query. How big is the MySQL database? Do you have the database schema somewhere? Otherwise, my input might be limited.

[dongskyler]

I think adding server-side caching is a great idea. But it's also a good idea to improve the efficiency of SQL queries

[dongskyler]

Hi @dongskyler! Sorry, i think we must've given you a mistake in our suggested code change! Thanks for looking into this! We SUPER appreciate your help, and please don't feel this was your fault in any way! 😄

Hmm. I wonder -- it's a pretty odd bug. The change was to add the .joins() statement on this line:

https://github.com/publiclab/plots2/blob/0e58e62edd5efe6c9f22fce6ac24bf770b932961/app/controllers/stats_controller.rb#L4

Is it possible we're joining incorrectly? What if we joined against something which has only 219 records... or something... ?

Thank you very much for your kind words. I'm just trying to contribute here.

[jywarren]

Sure, some docs here:

• https://github.com/publiclab/plots2/tree/master/db/schema.rb.example
• https://github.com/publiclab/plots2/blob/master/doc/DATA_MODEL.md

I opened a PR with the cached version. We have about 5000 tag_selections, maybe 10000 nodes, maybe 68k NodeTags.

[jywarren]

So, the PR passed, and I'm tempted to merge this for now, but if you LOVE optimizing SQL, we can leave this issue open and see if there's another way to do it more efficiently! I'm not such a Rails SQL wizard, so I guess it doesn't seem like a very friendly place to start contributing 😅 but for sure some folks are great at databases so don't let me scare you off of it if you are!

If you're looking for something else to get into, there are a bunch of other issues I might suggest!

[jywarren]

Gah! So my cache code must've broken... this loaded the first time: https://stable.publiclab.org/stats/subscriptions

but the second pageload it broke! Fixing it now.

[dongskyler]

I think it looks great. It works! Server-side caching is fantastic.

I have 2 comments:

1. Perhaps, 24-caching is a bit too long? Maybe set the cache to expire in 12 hours or even 6 hours?

2. On the https://publiclab.org/stats/subscriptions website, why tags are ordered in such an odd way? It makes it difficult to comprehend the information on the popularity of subscription tags.

The first 4 rows are:

Subscriptions | Tags
-- | --
160 - 169 | formaldehyde 164
0 - 9 | open-pipe-kit 1 openhour 1 opk 1
230 - 239 | water-quality 230
120 - 129 | bienvenue 129 flood 129 knowflow 129 oil-testing 129 otk 129 question:air-quality 129

It starts with 160-169 subscriptions, and then moves to 0-9, and then 230-239.

So clearly it's not in the order of increasing integers.

It's not alphabetical, either, although the tags are in alphabetical order within each cell.

My suggestion is to order Subscriptions in an increasing or decreasing integer order, such that it shows tags with 0-9 subscriptions, followed by 10-19, and so forth.

Or you can add a feature to let users choose the order (increasing or decreasing, by integer or by tag name, etc).

Attached is a screenshot of the current state of the webpage:

[jywarren]

And yes, the ordering is also a bug! We should definitely solve that too! decreasing integer order sounds perfect, though! I'd be happy to accept a PR for that.

In the meantime it doesn't look like it works on stable but I rolled back the node status check, so let's see: https://github.com/publiclab/plots2/commit/51d21a55dd07dabd7683175cdb7d22358a4d5423

Not happy with that, but not sure what's going on...

[jywarren]

And another quick fix just to remove the caching code: https://github.com/publiclab/plots2/commit/9c726fda8c7ac74066197078c6ae3819834440bb

[jywarren]

So to sum up, in part due to the caching code, the corrected fix is now commented out. It runs too slowly to turn on for every pageload, so if we can get the caching working, we can fix it.

https://github.com/publiclab/plots2/blob/9c726fda8c7ac74066197078c6ae3819834440bb/app/controllers/stats_controller.rb#L3-L17

We'd love your help, @dongskyler if you're interested!!!

[jywarren]

I'm not sure why the caching caused trouble. But we could cache in the view template instead, which is easy too: https://guides.rubyonrails.org/caching_with_rails.html

We would just wrap these lines:

https://github.com/publiclab/plots2/blob/9c726fda8c7ac74066197078c6ae3819834440bb/app/views/stats/subscriptions.html.erb#L14

[emilyashley]

Just checking in that this came up in our weekly staff call today. We're getting ready to onboard a new staff member and this view will be very relevant to them.

Bumping this up in priority!

Important counts we're looking at right now:

• number of people following a tag
• unique contributors per topic area
• total amount of content in a topic area (questions, notes, wiki pages)

[emilyashley]

Do we have breakout issues for

1. tags appearing that have not been used in content?

or

1. ordering the tags in order of count? (edit: I see it here #7934) sorry catching up

and

1. having the correct counts on tags

or are these included in this issue?

[jywarren]

https://github.com/publiclab/plots2/pull/7941 resolved the ordering issue. But i think we have the remaining issues, looking at https://stable.publiclab.org/stats/subscriptions:

• spam tags still seem to appear, for example tags with quickbooks in them
• the counts seem to be amplified. Those that ought to have the most do have the most, but spectrometer has 149526 but we don't have anywhere near that number of nodes, so i think some multiplication is happening somewhere, possibly in the table joining.

We'll spend some time digging in on this today!

[jywarren]

I think it's possible we need to create better test data for this so we can work the problem locally.

[jywarren]

I believe I've fixed the caching syntax and pushed a new PR. Let's fingers 🤞

[jywarren]

Merged, just need to confirm on https://stable.publiclab.org/stats/subscriptions now!

[jywarren]

OK, so, it's an improvement, but the counts are still inflated. For example, although https://stable.publiclab.org/tag/balloon-mapping is indeed a top tag, it has 80 followers, not 56700. I believe the number shown is not being grouped properly, and is repeating counts due to improper table joins and groupings. Next step is to debug on the console on the live server.

[jywarren]

OK, plan is to:

1. revert to original and just live with the spam
2. check the spam timestamps to see if it's even occurring anymore, maybe it was only possible during a specific window of time?
3. manually delete the spam

[jywarren]

OK so I have another attempt, manually tested on the live database, and I think this is correct:

https://github.com/publiclab/plots2/blob/9b469ee3b10c9c68ec51dea51a857efcdc6302d5/app/controllers/stats_controller.rb#L3-L9

Note "DISTINCT" - it returns 109 for balloon-mapping, which is correct.

And it mostly works! The stats look much better: https://stable.publiclab.org/stats/subscriptions

But, balloon-mapping still has 100 instead of 80, which is weird because https://stable.publiclab.org/tag/balloon-mapping shows 80.

Aha, i wonder... we aren't filtering out banned people. Let's look:

https://github.com/publiclab/plots2/blob/9b469ee3b10c9c68ec51dea51a857efcdc6302d5/app/models/tag.rb#L182-L186

[jywarren]

OK, added that too and re-added cache code, because that slowed it down a lot:

https://github.com/publiclab/plots2/blob/8b0b3d50ac7797a1c66bc9ecbfda53482dd31c5c/app/controllers/stats_controller.rb#L3-L13

[jywarren]

I think we're done here! Fully working on stable! Will publish soon.

[ebarry]

This page is looking great! https://publiclab.org/stats/subscriptions
I think we can close this issue

[jywarren]

Strangely this has re-broken, so re-opening:

8b0b3d50ac7797a1c66bc9ecbfda53482dd31c5c was the last fix, a day before we closed this and had confirmed it on stable.

It's no longer working on stable either: https://stable.publiclab.org/stats/subscriptions

[jywarren]

On stable:

counts = TagSelection.select("DISTINCT tag_selections.tid, tag_selections.user_id").where(following: true).joins("INNER JOIN community_tags ON community_tags.tid = tag_selections.tid").joins("INNER JOIN term_data ON term_data.tid = community_tags.tid").group("term_data.name").joins("INNER JOIN rusers ON rusers.id = tag_selections.user_id").where("rusers.status = 1 OR rusers.status = 4").count["balloon-mapping"]
=> 88

which seems correct...????? vs. 61336 on https://stable.publiclab.org/stats/subscriptions ???

[ebarry]

Just came across this myself today on https://publiclab.org/stats/subscriptions. Compare the difference between what's listed at that URL vs on the Tag's own page:

• 
• 

[jywarren]

I recall someone suggesting we need much better test data to reproduce this and I wanted to suggest we develop some code snippets that can be used to generate a ton of users, tags, and subscriptions, to try to simulate the failure.

[17sushmita]

Part of #9827