#metabrainz

ruaok, _lucifer: is this something that is easy to do now? https://tickets.metabrainz.org/browse/MBS-11586

MBS-11586: Show How Many Times a Recording Has Been Listened to with ListenBrainz on Recording's Page

reosarevok: No, I think it'll be difficult to do. one major is issue is that listens are linked to a `msid` not a `mbid`.

secondly, i am not sure but as per the current structure of the data, such queries might be expensive.

zas: ruaok: the worker nodes are failing some jobs due to lack of disk space. I see the nodes have a lot of free space but during processing some data is stored at `/tmp`. this path property is configurable. should i point the nodes to store data somewhere else like `/data/tmp` maybe?

the persistent data is being stored at `/data/datanode` and `/data/namenode`. let me know i should modify that as well.

_lucifer: yes. exactly that.

/data seems fine

also, moooin!

good morning!

moin, how are things in dublin?

it's been sunny this week!

said like a true northerner. :)

hello

ruaok: interesting to see that the materialised view also has lots of subtables

how many rows does listen_count_5day have? hypothesis: if it was just a plain old postgres table with a bunch of indexes, would it be faster?

yeah. the thing that flummoxxes me is that it a search on the materialized index with start and end constraints still does a full index scan on all chunks. which kills performance.

> if it was just a plain old postgres table with a bunch of indexes, would it be faster?

it should be faster. right now it has 2M rows or so. with a proper index it is faster.

my conclusion: cont aggs are too critical to be a mission critical element of our infrastructure.

but, I have a plan!

yeah, I was surprised to see all of those scans on each chunk, when I guess we put all of this effort into minimising the number of chunks that we wanted to scan back when we started this optimisation process

first, we should store listen count, min ts, max ts in redis -- perhaps even one key since they often are needed in the same place. Since we have ONE place where we update the listen table (timescale_writer) we can ensure that the values in redis are kept accurate. And this has been working well from what I can tell. So, lets not expire those keys anymore -- primary store is in redis, but can be recomputed when needed by expiring the

keys.

ruaok: 👍 , i'll restart the cluster after changing the local dir and we should be able to do more requests.

that cuts out a 2.7s query. not hard to accomplish.

right, so instead of having to do a scan, be it indexed or table scan, we just do a redis get

great

btw, i checked it did process some stats jobs yesterday.

next, I took a look at our usage patterns. and I think we should make sure that fetching recent listens is FAST and older listens reasonably fast. ancient listens can be slower. but most of our queries will be recent shit, so make them fast.

I'm finishing debugging an approach that starts with a 30d window and then does an exponential backoff on the query bounds.

great, that makes sense too

if not found in 30 days, save those results and adjust bounds to be wider and just outside the last bounds.

my goal is to fetch listens in under 100ms on avg.

hopefully better than that.

hopefully we can test that in a minute.

does any of that sounds like a bad approach?

furthermore, cont aggs don't support month buckets for the data view you and Mr_Monkey wanted.

but I created a 30 day bucket, which will never align right, but get the overall job done -- a month or year aligned query on top of that, will still fetch the data in a reasonable manner.

30 day bucket cont agg.

will the exponential backoff still hit chunk boundaries?

no.

but the idea would be that the first few chunks will get most of the data that we want anyway, so it should be fast?

yes, exactly that.

see the paste for how the timestamps change over time. right not I have it tuned to give max 3 queries.

yeah right, nice

yisss. tests pass.

one question: from the pg explain above, do you know what `listened_at_bucket < COALESCE(_timescaledb_internal.cagg_watermark('35'::oid), '-9223372036854775808'::bigint))` is?

I assume that this is some internal timescale thing, but it's interesting that the bigint argument is the same on all chunks

oh yes, and when we cache timestamps, then the lower bound of the query will be the timestamps of the user, further making it more efficient. (new users will never paw through 20 years of data)

I don't know. has confused me too.

We used to cache min max timestamps during Influx time iirc

we still may. I just want to shore them up and store them in one redis key.

I haven't dealt with that yet.

awww yiss: `2021-04-16 09:30:38,546 INFO fetch listens 0.03s in 1 passes`

fetch this: https://test.listenbrainz.org/user/mr_monkey

That looks goood

Damn thats atleast 2x faster for me

yes, we clearly cache timestamps. once they are in cache, things are fast.

30ms. nice. I was shooting for under 100ms.

yesterday was a loong day, but in the end worth it.

Strange enough that the cont. agg. Is so slow

sweet, nice work

wooooo, wooo fucking hooo!

man, that was a long slog.

!m ruaok

You're doing good work, ruaok!

But the performance boost is cleary noticable.

anything that is slow, is the ts fetch. once that is in cache, booom!

:D

[listenbrainz-server] mayhem opened pull request #1390 (master…be-gone-time-ranges): Remove time ranges and refactor listen fetching https://github.com/metabrainz/listenbrainz-serv...

alastairp: hi! regarding https://github.com/metabrainz/listenbrainz-serv..., do you mean to reject invalid format tags or just not build and push the image to docker hub?

good question

can you reject badly formatted tags at a github level?

I think it's fine to just match this action on a regex

alastairp: could you please glance at https://github.com/metabrainz/listenbrainz-serv... and tell me if we should finish this and merge or if I should do the timestamp improvements before finishing this PR?

Nice work ruaok !

I would prefer to finish the timestamp improvements in the same PR.

Mr_Monkey: thanks. :)

Mr_Monkey: please check the branch to see if you find any issues.

Looking now

the PR is already long, but mostly because of deleted code.

We're talking of PR 1390 you posted above, correct?

yes

which is live on test

did you read my comments on the month and year aggregated views?

alastairp: i don't we can reject such tags directly, we'd need to write some webhook and wire BrainzBot maybe. probably not worth it. the other solution to match the regex is easy. yup, let's do that.

ruaok: a quey about that PR: `CREATE VIEW listen_count_5day WITH ...`. I'm not super familiar with this part of postgres

this is a materialised view (I understand yes, because you can only create indexes on materialised views?), in that case, what's the difference between CREAET VIEW and CREATE MATERILIZED VIEW?

that part will likely still change. I've not settled on what to do yet.

but I may just replace the listen_count with listen_count_30days and listen_count_365days.

zas: https://blog.mozilla.org/addons/2021/04/15/buil...

outsidecontext: https://data.musicbrainz.org

alastairp: thats how things are done in TS 1.7. IN 2.x+ you create a materialized view. I suspect that an upgrade will help us on a few fronts, but they still won't make the approach from yesterday viable.

zas: oh, great. so we could already change the links for the picard download page?

ruaok: ok, thanks for clarifying

!m zas

You're doing good work, zas!

outsidecontext: I guess so, I need have few bits to tune, but it is here to stay

alastairp: so thoughts on continue vs new PR for the timestamps work?

ruaok: I'm reading through the PR now

cool. shall I submit a PR for the website?

for now, it supports http & https, should I just redirect http to https?

zas: I don't think so.

k

ruaok: can you clarify - this PR includes the changes that we discussed on wednesday: adds the 5 day aggregate, removes the need for making multiple queries on the frontend. query times are on par with what they were before this change. your new proposed improvements include caching of the min/max and the increasing range query, which gets us down to your example 30ms query?

correct. however, the "adds the 5 day aggregate" will likely still change a bit.

it is no longer crucial to have the cont agg in lock step with the hypertable.

I am learning toward replacing the listen_count with listen_count_30day, rather than creating a new listen_count_5day.

it should make fetching timestamps a bit faster.

so the 30day cont agg and the timestamp work are the only bits left.

ah, I see. this is why it also makes sense to merge your timestamp work into this same PR

yes.

and then we have one larger PR, but this whole whicket of issues gets solved.

and the basis for 30day listen overview is in place for future work.

this PR is already quite large, though I see a lot of it is just changing arguments in tests.

the timestamp stuff is going to be pretty self-contained in a single file, right?

exactly why I am asking before proceeding.

2, but year.

yeah

I will create a new agg in this code, but we wont need an SQL update script yet. I've done it by hand.

go ahead with putting both in this PR then, as the timestamp stuff isn't going directly affect the other 21 files

once deployed I will open a small PR for removing the old cont aggs.

I've got a meeting now, but after lunch I'll be available for review

kewl. I'll finish that this afternoon with some luck.

I suspect that PR wont be ready until monday, so no rush.

ok cool

I worked from 10am until 1:30 in the morning with few breaks, so I'd like a bike ride this afternoon. :)

can I pre-review this? I see a few suggestions already, or would you prefer to wait until you finish?

theoretically comarca restrictions get lifted on Monday!

don't go digging into too much detail and skip comments on the SQL scripts, but yes feedback would be welcome.

ohh, really?? that would be fantastic.

to "Veguerias", which are circa 14th century political divisions, lol

so our one includes Barceloneces, Maresme, and the 2 Valles

"the feudal administrative territorial jurisdiction of the Principality of Catalonia (to the Crown of Aragon) during the Middle Ages and into the Modern Era until the Nueva Planta decrees of 1716."

crap.

oh well, a trip up to Arenys would also be nice.