#metabrainz

/

      • ruaok
        build fixed?
      • Mr_Monkey
        ruaok: Yes. I just pushed an updated and rebuilt image of PR 1390 for test.lb if you want to deploy it.
      • I'm now looking at front-end tests that I borked during a merge.
      • alastairp
        _lucifer: yeah, look at every other dockerfile in metabrainz that uses a pre-defined ARG in a LABEL, and you'll see that yvanzo follows this exact patter
      • its... weird. but apparently they way that it has to work
      • oh, did _lucifer fix the babel thing??!?
      • !m _lucifer (and Mr_Monkey, I guess)
      • BrainzBot
        You're doing good work, _lucifer (and Mr_Monkey, I guess)!
      • Mr_Monkey
        I fixed it in another way that a drawback, so not as good :p
      • that added a drawback*
      • alastairp
        weiiird
      • rebasing and testing 1424, then
      • Mr_Monkey: oh, I ran into something testing on my other computer. new docker uses this thing called "buildkit", and it doesn't leave behind intermediate images like old "docker build" does, so this issue that we had about having old LABELs that don't make sense isn't an issue anymore
      • because those intermediate layers appear to no longer exist
      • Mr_Monkey
        Huh
      • But there's no ill effects to setting it to blank?
      • alastairp
        no. I've left the "setting it to blank" in place until I can work out how buildkit fits into the grand scheme of things
      • It'll probably become a defult in future versions of docker, but until then let's keep the workaround in place
      • BrainzGit
        [listenbrainz-server] alastair merged pull request #1424 (master…add-git-hash-label-last): Don't add GIT_COMMIT_SHA build to the label at the beginning of the build https://github.com/metabrainz/listenbrainz-serv...
      • alastairp
        _lucifer: ^ fixed - can you please add the tricks for the duplicate ARG to the spark PR?
      • there's now a conflict in https://github.com/metabrainz/listenbrainz-serv..., I'm fixing it
      • ShivamAwasthi joined the channel
      • _lucifer
        alastairp: thanks! i'll add the label thing to spark PR as well
      • BrainzGit
        [listenbrainz-server] alastair merged pull request #1430 (master…gh-prod): LB-882: Build production image in Github actions https://github.com/metabrainz/listenbrainz-serv...
      • ruaok
        PR 1390 is built with 'test' 'test', Mr_Monkey ?
      • Mr_Monkey
        Correct
      • ruaok
        thx!
      • ShivamAwasthi has quit
      • alastairp: you about?
      • _lucifer
        ruaok: alastairp: can you please take a look at this script to copy emails from MB db to LB? https://github.com/metabrainz/listenbrainz-serv...
      • alastairp
        _lucifer: in what context will this be used? daily updates? one-off? is it associated with a PR?
      • ruaok: hi
      • _lucifer
        alastairp: this will be run by the cron job to update users emails daily
      • ruaok
        looks good. but, I do wonder if this should run in a transaction.
      • in case something biffs, you can try again without making a mess.
      • alastairp
        do we think it's OK to do it in bulk on all users each time? Or only the users whose email address has changed?
      • ruaok
        ok today, in a few years? prolly not.
      • but if this script goes away once we have centralized meb accounts, then this ought to be fine.
      • alastairp
        good point
      • _lucifer
        yeah, this should go away at that time.
      • ruaok
        add a transaction, then I'd be happy in that case.
      • alastairp
        _lucifer: you don't need ; to terminate sql statements in ptyhon
      • _lucifer
        alastairp, yeah i know. i added to separate the quotes.
      • alastairp
        ah, right. maybe a space?
      • or quote the string with ''
      • _lucifer
        sure, will do that.
      • ruaok
        I have a puzzle for you two, _lucifer & alastairp.
      • alastairp
        I've not used UPDATE .. SET FROM before
      • ruaok
        there are no keys in redis. but test.lb.org is clearly pulling items from the cache.
      • I checked IP and port and they match.
      • but NO keys?
      • alastairp
        test.lb? first thing to check, is it the same redis instance?
      • _lucifer
        maybe its treating * as the keyname
      • yeah try it here, it returns nil
      • alastairp
        isn't there a KEYS command too to get the keys?
      • ruaok
        yes. test. I checked that it connects to the right version.
      • keys works. but not get.
      • ruaok wonders if it used to.
      • redis has been running for weeks.
      • ruaok shrugs
      • _lucifer
        it should be KEYS *
      • ruaok
        sorry, then. just being stupid. as usual.
      • _lucifer
        happens to me all the time :)
      • Mr_Monkey
        Yeah, how can you not perfectly remember the million commands from a hundred languages and syntaxes?
      • ruaok
        I can't even remember them from week to week.
      • but strncpy(dest, src, num) will NEVER fall out of my brain.
      • I'll be in the old folks home, not remembering shit... Rob: Arguments to strcnpy, go!
      • des, src, num
      • who am i?
      • _lucifer
        lol, i always get confused whether its dest, src or src, dest
      • Mr_Monkey
        "Is my name desrcnum"?
      • ruaok
        dest src. matches x86 assembler convention.
      • shivam-kapila
        Assemblers. Ah shit here we go again
      • ruaok
        WOW.
      • ts 2.2.0 IS a lot faster.
      • let me remove all the cache keys for the new setup (they are all stale now)
      • _lucifer
        alastairp: my ide is issuing a warning if i leave the org.label-schema.vcs-ref= empty. safe to ignore?
      • alastairp
        _lucifer: good question. the build works, so 🤷...
      • maybe we can set it to "" ?
      • _lucifer
        yeah that quitens it
      • alastairp
        ruaok: at least you're not using strcpy
      • ruaok
        there lie daemons.
      • alastairp
        great to know that 2.2 is faster! possible that we can remove the redis keys?
      • ruaok
        with their ttys properly detached and stdin/stdout closed.
      • keeping the keys will still be more future proof. but recovery from losing them might not be as dire.
      • _lucifer
      • attempt to store cache got ratelimited
      • alastairp
        the post-run logging looks interesting
      • _lucifer
        alastairp, updated https://github.com/metabrainz/listenbrainz-serv... with the label, want to take a look again or should i merge
      • alastairp
        nable to reserve cache with key layer-docker-layer-caching-ListenBrainz Unit Tests-d261fe9be9433846e3ce7055d0a028c5a11ad8e664d1ce7e81b5e00730bb7eea, another job may be creating this cache.
      • _lucifer
        ah my bad
      • alastairp
        you need to add `ARG PYTHON_BASE_IMAGE_VERSION` after the FROM line but before LABEL
      • _lucifer
        one sec
      • alastairp
        maybe we should also add org.label-schema.vcs-ref at the end of the build as well? to be consistent with the other LB images
      • _lucifer
      • its added only to the prod image only to avoid invalidating the entire test setup.
      • alastairp
        right, nice
      • one thing - I've encountered an issue before in docker multistage builds where in a file like this, if you ask it to build test then it'll also do the items in prod
      • I don't know if this is something that still happens. did you see anything like it?
      • (sorry, I missed the fact that this was a multi-stage file)
      • _lucifer
        yeah, i noticed that. but its only one step so i let it slide.
      • alastairp
        run me through - why does the prod version not need java, but the test one does?
      • _lucifer
        the prod version has java, spark and hadoop installed directly on the node
      • we only use docker to manage python dependencies
      • but i think i might be able to get that done without the docket image soon, at that point entire prod setup will be dockerless
      • alastairp
        and you build a docker image with the dependencies and then copy them out of the docker image into the spark environment?
      • yeah, I think it makes sense to either run everything in docker, or nothing. but we should also think about consistency - if local dev uses docker and production doesn't, is there a risk of things working in one and not the other?
      • _lucifer
        no the spark installation is mounted to the docker image so it runs inside the container
      • yeah, the current setup is a bit of a hack.
      • alastairp
        I see. what was the reason for that?
      • _lucifer
      • the prod is setup as hadoop yarn cluster. every node has hadoop, yarn and spark configured.
      • on the leader node (the driver) we run the spark-request-consumer inside a docker container.
      • one of the reason for that is historically everything was run inside docker.
      • every node needs python deps to run the code. the script i shared above does the task to distribute it to the worker nodes.
      • L12-L16, install all the python deps we need in a virtual env and distribute it to the workers as a zip file
      • however, it does distribute this file to the driver
      • alastairp
        right, and there's not an easy way of running each node as a docker container which includes the source and the dependencies?
      • I don't know anything about hadoop, spark, or yarn, so I'm just making stuff up. If this is the way it has to work then that's fine
      • _lucifer
        that's intentional, we do not want to run inside docker due to security concerns
      • zas or ruaok can explain those better than me. but at the end we want to restrict docker to a minimum on the cluster.
      • the odd one out is the spark-request-consumer, i want to get docker out of the equation there as well
      • there shouldn't be docker here at all.
      • alastairp
        how does the cluster work currently with new code / new dependencies?
      • you build a new image from Dockerfile.spark and do something with that start script?
      • what would the new way be to do it?
      • _lucifer
        exactly the same, just without the docker run line
      • the driver also gets the deps as the zip file or from the venv
      • alastairp
        but how does the updated code get into the cluster?
      • _lucifer
        its also packaged as a zip, on the leader node we have a clone of the lb repo
      • alastairp
        right. so ssh leader; git pull; run some command ?
      • _lucifer
        yup, ssh, git pull, the script above, done.
      • alastairp
        so the only reason that there's a dokerfile is so that we can run `spark_manage.py request_consumer` ?
      • and an alternative would be to just run that from within the pyspark_venv env?
      • _lucifer
        yes
      • ruaok
      • alastairp
        yeah, especially because there's a (small) risk that the env in the docker image and in the packed venv could be different, I think that's a good idea
      • ruaok
        much much much better with ts 2.2
      • alastairp
        and then we'd still have a dockerfile-based setup for local development?
      • _lucifer
        yeah, right.
      • alastairp
        cool, sounds useful. thanks for you work!
      • _lucifer
        spark is agnostic to the underlying setup so the code does not needs to change.
      • alastairp, if you are available, let's test the AB on bono? i tried to bring up the instance but was unable to do it.
      • alastairp
        _lucifer: right - sorry, this was a bit messy because I always thought it was going to be temporary
      • there's a bono.sh file, this uses docker/docker-compose.bono.yml to build, and then docker-compose.yml to start nginx + uwsgi
      • _lucifer
        right, i saw that. it says acousticbrainz-server in the image but there's no such image methinks.