Feature - Background Queue Worker / Highscore

[jackbayliss]

WIP for #417

• Adds new scheduler docker image that uses the same image as the main
• scheduler image called the php artisan schedule command every minute
• You can use the console.php route, to pass in commands to run - example is in there for now-- will be removed.

To do:

• Add background queue worker
• Fix tests
• Sort out the high score command
• Resolve PHP stan / Pint issues
• Add command to the console.php
• Ensure it runs as expected, ie every 5 minutes.
• Adjust current logic to use DB- and ensure it's cached. (Potentially add a rank column for each type, so we don't have to do anything!

[lanedirt]

Hi @jackbayliss,

Thanks for taking on the work for this issue! The new scheduler container looks like a nice and flexible way to approach this, nice!

I'm not sure exactly why the tests are failing, however I noticed that you made a change to the Dockerfile that uses the following.

USER root

To share some helpful context: when working on implementing the GitHub actions before I learned that GitHub actions have a restriction in that it doesn't support USER statements. You can read more about it here:
https://docs.github.com/en/actions/sharing-automations/creating-actions/dockerfile-support-for-github-actions

In order to fix this issue I added a step to the GitHub actions that strips out the original USER www command:

.github/workflows/run-tests-docker-compose.yml

    steps:
      - uses: actions/checkout@v2
      - name: Modify Dockerfile to disable USER www (which is not supported by GitHub Actions, as it runs as root)
        run: |
          sed -i '/USER www/s/^/#/' ./Dockerfile
      - name: Set up Docker Compose

So this might be the reason for why it's failing now since the string search and replace cannot find the original string anymore.

Additionally with this USER root line change the actual application will now also run under root which is not a best practice, so it might be good to revert back to the www user before starting the actual application like so:

USER root
COPY docker/entrypoint.sh /usr/local/bin/entrypoint
RUN ["chmod", "u+x", "/usr/local/bin/entrypoint"]
# Switch back to www user for running the application
USER www
CMD ["/usr/local/bin/entrypoint"]

BTW as this is a WIP feature, feel free to disregard my comments if you've already looked into this! Just trying to give some context in case it helps! 😄

[jackbayliss]

@lanedirt appreciate the heads up!

Still quite a bit to do, but wanted to get the new container scheduler approach up just incase anyone wanted to work on the high score bit / get some feedback, but glad you think it works.

It seems to work well so far locally, but hoping to continue and clear everything up at some point over the weekend / this week

Thanks for the insight 😃

[jackbayliss]

Tests are now fixed 🚀 🐛

I had setup the dev docker image, not the prod, have now sorted the prod image-- prod image was used by the actions.

[jackbayliss]

More WIP changes, PHP Stan has come up with an error due to using chunkbyId I believe, so I need to resolve that. but a good start for now- once I sort that we should be able to switch out the service to use the DB rather than calling it on the fly... which means we could then use pagination.

[jackbayliss]

@lanedirt This should finally be good for a bit of testing.

It's using the DB and caching various bits so should be an overall improvement.

One thing I noticed is PHPStan was having a issue with the chunkById function or at least I think it was. Reason for using it, is rather than loading everything in, it uses LIMIT etc in the background it chunks it and makes it more efficient. I've had to ignore the lines it was borking on, but you may know a better way around it!

The good thing about the schedule is, in future if there's thousands of users - it will call it every 5 minutes, but take as long as it needs to finish the command. But can always increase it in future.

Side note, I've noticed the UserFactory could do with updating to match the current user fields.

Still could do with some tests being added, but at least you can see it and how it works 😄

I've added a new Highscore Model, that uses a scope - this just means we can easily access the scope when using the model-- ie ValidRanks.

I may be unavailable for the new few days due to work commitments, so do feel free to make amends if it's urgent 👍🏻

[lanedirt]

Hi @jackbayliss,

What can I say: awesome work, really cool! I can learn quite a few things from this 😄! Thanks for your persistence on implementing this feature.

I’ll go through the changes in the coming days and test them in my local environment. I’ll also try testing it with a copy of the current main.ogamex.dev production database to see how it works with that dataset (around 150-200 users).

I’ll let you know once I’ve had a chance to test & review it.

[lanedirt]

Hi @jackbayliss,

Thanks again for your work on this. I tested it on my localhost and came across a few things. I also added some minor comments inline to the PR. Other than these things, the changes look good to me in terms of structure. 👍

1. When I start the updated docker stack via docker compose up -d the scheduler container comes online, however I'm not seeing any highscores appear. The highscore list stays empty on my environment. I waited for 10+ minutes to ensure any cache should have been cleared. The scheduler docker container is not showing any errors, however I'm also not seeing any confirmation that the scheduler is running. Am I missing a step by any chance?

$ docker compose logs scheduler
scheduler-1  | [02-Nov-2024 13:56:12] NOTICE: [pool www] 'user' directive is ignored when FPM is not running as root
scheduler-1  | [02-Nov-2024 13:56:12] NOTICE: [pool www] 'group' directive is ignored when FPM is not running as root
scheduler-1  | [02-Nov-2024 13:56:12] NOTICE: fpm is running, pid 1
scheduler-1  | [02-Nov-2024 13:56:12] NOTICE: ready to handle connections

2. When I manually run the artisan commands I do get contents on the highscore page. This looks pretty good, however with the production dataset of 200+ players the sort does not seem to work correctly. On page 2 where all players have a score of 0, suddenly on rank 200 I see two players with a positive score. This is what I see:

3. I also noticed something in the paging that seems to be off. Page 1 shows ranks 1 - 102 for me. I would expect it to show ranks 1-100, for exactly 100 players per page.

4. Question for new player registrations: when a new player creates an account, currently because of the 5 minute delay in cache they do not yet show up in the highscores and the highscore rank shows 0, screenshots below. The official game has a certain mechanism where if a new player creates an account they do show up immediately in the highscores with score "0", and they can see their actual rank also in the top menu above. Is it possible to integrate this logic into the new highscore system, i.e. when a new user creates their account they get an associated highscore record in the db where their rank is simply current last rank + 1, and all points set to 0, and caches are cleared?

Could you have a look at these comments? Thanks a lot! 🚀

[jackbayliss]

@lanedirt Thanks! I'm not sure why the scheduler isn't working for you, did you a try a docker compose up -d --build if it works, you should see it outputs if you exec into that container. I will check it over locally again though using a fresh build.

As for new registrations, we could use an observable to listen on user created, so whenever it's triggered it'll run the commands or as you say take the last max and add 1

I only had two users locally, I'll have to play with the factories to try and generate some fake scores to adjust the logic. As I don't have the users you do.

Comments all look good, minor adjustments - thanks for looking into the Stan thing 🙂

[lanedirt]

@lanedirt Thanks! I'm not sure why the scheduler isn't working for you, did you a try a docker compose up -d --build if it works, you should see it outputs if you exec into that container. I will check it over locally again though using a fresh build.

Yes the --build seems to have done the trick. 👍 Good catch. Makes sense, as it probably used a cached build of the main Dockerfile which didn't had the entrypoint logic yet. Thanks!

Now however when I run the docker compose logs I get a permission denied error:

$ docker compose logs scheduler
scheduler-1  | /usr/local/bin/docker-php-entrypoint: 9: exec: /usr/local/bin/entrypoint: Permission denied

Do I need to make this file executable manually or is something missing from the Dockerfile logic? Because the user/group permissions seem to be fine. They are all set to user:staff on my local Mac.

$ ls -hal
total 8
drwxr-xr-x   3 user  staff    96B Nov  2 14:48 .
drwxr-xr-x  52 user  staff   1.6K Nov  2 14:48 ..
-rw-r--r--   1 user  staff   337B Nov  2 14:48 entrypoint.sh

[jackbayliss]

@lanedirt Thanks! I'm not sure why the scheduler isn't working for you, did you a try a docker compose up -d --build if it works, you should see it outputs if you exec into that container. I will check it over locally again though using a fresh build.

Yes the --build seems to have done the trick. 👍 Good catch. Makes sense, as it probably used a cached build of the main Dockerfile which didn't had the entrypoint logic yet. Thanks!

Now however when I run the docker compose logs I get a permission denied error:

$ docker compose logs scheduler
scheduler-1  | /usr/local/bin/docker-php-entrypoint: 9: exec: /usr/local/bin/entrypoint: Permission denied

Do I need to make this file executable manually or is something missing from the Dockerfile logic? Because the user/group permissions seem to be fine. They are all set to user:staff on my local Mac.

$ ls -hal
total 8
drwxr-xr-x   3 user  staff    96B Nov  2 14:48 .
drwxr-xr-x  52 user  staff   1.6K Nov  2 14:48 ..
-rw-r--r--   1 user  staff   337B Nov  2 14:48 entrypoint.sh

I'm not sure - I usually exec into the image and watch the log that way 😀

[jackbayliss]

Most recent changes should have sorted your comments!

I've made sure now when a user registers, they'll automatically get added to the highscore board. This hinges off the user tech model for now - but can easily change it to the user one. I felt the user tech model was better, as the PlayerService requires tech on the high score.

I've altered the queries so it should now page correctly it was pulling 100 entries correctly, but the issue is some rankings were missing where the users didn't have tech etc.. at least that happened locally. Let me know if it's still not right for you? Not sure there's a factory to easy generate this.

I've looked at the docker compose logs scheduler command, and it doesn't happen for my locally (on Windows) - it appears as expected, so I t believe it's permission related.

[lanedirt]

Hi @jackbayliss,

Thanks for fixes! I just tested it again on my local environment however it looks like I'm still running into a few issues.

1. I'm still getting the permission denied error for the entrypoint. I tried to debug this issue on my end and on Linux/MacOS host systems the issue seems to be caused by the owner permissions for the entrypoint after it is copied. I tweaked the Dockerfile and with the following change it works for me. Could you test if the following also works for you on Windows?

# Switch to root to copy entry point scripts
USER root

# Copy entry point and set permissions for www user
COPY docker/entrypoint.sh /usr/local/bin/entrypoint
RUN chmod +x /usr/local/bin/entrypoint && \
    chown www:www /usr/local/bin/entrypoint

# Switch to www user
USER www

# Run entrypoint
CMD ["/usr/local/bin/entrypoint"]

2. I'm still getting rank number 1-102 on the first page with production data while I would expect 1-100. This was with production data as-is, however after I ran the test suite locally (which creates new users) the list changes even more and I get rank 1-197 on the first page. See screenshots below.

PS: if you want to add more test users to your local environment you should be able to do so by running the test suite locally with php artisan test. With the amount of feature tests that we have currently every test run should add approx 50-100 users to the local database. Perhaps this will allow you to reproduce the behavior I'm seeing below.

3. When I create a new player now I do see that the highscore link in the topmenu shows the correct rank, which is superb! However when I then click on the highscore page I cannot see myself (yet). I guess this probably has to do with caching?Would it be possible to add a cache clear for the highscore list in the observer you created? Reason for this is because I know from my own play sessions that whenever I start playing on a server I usually check the highscore page to see how much players there are and how many points they have. So it would be a nice user experience if the highscore page directly works for the new user as it normally would. Example of what I see as a new player:

[jackbayliss]

@lanedirt Thanks! I'll check it out. I presume you did the production DB from fresh? ie copy it into the repo and ran a migration?
Just checking you didnt use the same one as yesterday.

[lanedirt]

@lanedirt Thanks! I'll check it out. I presume you did the production DB from fresh? ie copy it into the repo and ran a migration? Just checking you didnt use the same one as yesterday.

Thanks! Yes correct, I started the test today with a fresh copy from production and then applied the migrations. I checked and there were no artifacts remaining from the previous test.

[jackbayliss]

I've made some adjustments to sort the new user bits, and the docker image stuff- I ran the tests and the ranks seem to match fine - see screenshot (ignore weird formatting, its because ive done it in chrome)

I did run php artisan ogamex:generate-highscores and php artisan ogamex:generate-highscore-ranks via CLI on the app container.

I will probably pick this up again over the weekend, so no rush. It may be worth sending me a copy of your .sql (ofc not the production one) if it still occurs, as I must be doing something 👍🏻

[lanedirt]

Thanks for the fixes. The new player registration flow looks to be working good now, thanks!

I checked the player ranking bug again. I could not reproduce it anymore with test data but with the production version I still got the weird paging results. I tried to do some debugging myself to help out and this is what I found.

This is what the database looks like after the highscore actions have ran. This shows records in the highscore table sorted by general points DESC.

What you can see here is that record 7 has general_rank 201 while it should be 7. The fact that its the first rank after 200 it made me check the code and especially the chunkById() part in app/Console/Commands/GenerateHighscoreRanks.php:

        $query = Highscore::query()->whereHas('player.tech')
        ->where($type->name, '!=', null);
        $query->orderBy($type->name, 'DESC');
        $bar = $this->output->createProgressBar();
        $bar->start($query->count());

        $query->chunkById(200, function ($highscores) use ($type, &$bar, &$rank) {
            /** @var \Illuminate\Support\Collection<int, Highscore> $highscores */
            foreach ($highscores as $highscore) {
                $highscore->{$type->name.'_rank'} = $rank;
                $highscore->save();
                $bar->advance();
                $rank++;

            }
        });

I checked the docs and it seems the chunkById() method does not play well with the ORDER clause in the query itself. I noticed that there is also a regular chunk() method so I tried to use that one, and after I ran the highscore actions again the ranks now seem to work properly.

This is what the database looks like with the chunk() method:

Could you check if using chunk() also works properly for you?

[jackbayliss]

@lanedirt Good shout! - It works as expected locally using the testing users. I've also rebuild the docker image and it works correctly for me on Windows just to double check - output of scheduler below :

[lanedirt]

Looks good to me!

Will merge the changes now and will then verify on main.ogamex.dev if everything works as expected. Major thanks for your work on this! 🚀

[jackbayliss]

Looks good to me!

Will merge the changes now and will then verify on main.ogamex.dev if everything works as expected. Major thanks for your work on this! 🚀

Awesome! Let me know if anything strange happens, I'll probably open an issue for some efficiency that could be made in the observer when I get chance.