Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement required interactions with beatmap mirrors #14

Merged
merged 4 commits into from
Dec 30, 2024

Conversation

bdach
Copy link
Contributor

@bdach bdach commented Dec 24, 2024

The "required interactions" in question here are mostly just purging outdated cached packages.

Was tested to work in a full-stack local environment involving osu-web, a local mirror, and s3.

@bdach bdach self-assigned this Dec 24, 2024
Comment on lines +47 to +48
// TODO: log error
return null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would ask to look away for now, my next item on the list (one of the very few remaining, actually) is an extensive global pass on observability, reliability, and logging.

public static async Task MarkPendingPurgeAsync(this MySqlConnection db, osu_mirror mirror, uint beatmapSetId, MySqlTransaction? transaction = null)
{
await db.ExecuteAsync(
"UPDATE `osu_mirrors` SET `pending_purge` = CONCAT(IFNULL(`pending_purge`, ''), @beatmapset_id) WHERE `mirror_id` = @mirror_id",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a , as part of the concat.

Reference:

$conn->exec("UPDATE osu_mirrors SET pending_purge = CONCAT(pending_purge, '{$beatmapSetId},') WHERE mirror_id = {$m['mirror_id']}");

also according to production, pending_purge cannot be null so you should be able to drop the IFNULL:

-- auto-generated definition
create table osu_mirrors
(
    mirror_id        tinyint auto_increment
        primary key,
    base_url         varchar(255)                              not null,
    traffic_used     bigint(11)    default 0                   not null,
    traffic_limit    bigint(11)    default 0                   not null,
    secret_key       varchar(50)   default 'osudownloadm1rr0r' not null,
    provider_user_id int unsigned  default 0                   not null,
    enabled          tinyint       default 1                   not null,
    version          decimal(4, 2)                             null,
    pending_purge    varchar(8192) default ''                  not null,
    perform_updates  tinyint       default 1                   not null,
    regions          varchar(8192)                             null,
    disk_space_free  bigint                                    null,
    is_master        tinyint(1)    default 0                   not null
)
    charset = utf8mb3
    row_format = DYNAMIC;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a , as part of the concat.

Good spot 😬 45137e9 should do it...?

also according to production, pending_purge cannot be null so you should be able to drop the IFNULL:

Interesting. On all my local environments it's nullable.

CREATE TABLE `osu_mirrors` (
  `mirror_id` tinyint NOT NULL AUTO_INCREMENT,
  `base_url` varchar(255) NOT NULL,
  `traffic_used` bigint NOT NULL DEFAULT '0',
  `traffic_limit` bigint NOT NULL DEFAULT '0',
  `secret_key` varchar(50) NOT NULL DEFAULT '',
  `provider_user_id` int unsigned NOT NULL DEFAULT '0',
  `enabled` tinyint NOT NULL DEFAULT '1',
  `version` decimal(4,2) DEFAULT NULL,
  `pending_purge` varchar(6000) DEFAULT NULL,
  `perform_updates` tinyint NOT NULL DEFAULT '1',
  `regions` varchar(6000) DEFAULT NULL,
  `disk_space_free` bigint DEFAULT NULL,
  `is_master` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`mirror_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci

Probably something or other with osu-web's autogen'd schema definitions not being 100% there on nullability...

@peppy peppy merged commit 42064f6 into ppy:master Dec 30, 2024
4 checks passed
@bdach bdach deleted the mirrors branch December 30, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants