Erreur : entier hors de portée

Bug
22

Cela a fonctionné pour les post_actions, merci pour la suggestion !

Nous constatons principalement encore deux échecs :

  1. GET https://devforum.roblox.com/notifications renvoie une erreur 404 pour les utilisateurs qui ont des notifications avec des identifiants bigInt.

  2. Le job planifié Jobs::GrantAnniversaryBadges échoue.

Est-il possible qu’un modèle/contrôleur/service de notifications soit toujours codé en dur pour définir le champ notifications.id comme un entier ?

23

très peu probable… 404 ne semble pas correct, y a-t-il quelque chose dans /logs qui décrit le problème ?

24

Malheureusement, je ne trouve rien de particulièrement utile dans /logs lui-même, mais lorsque je reproduis cela localement, je vois la requête arriver puis un 404, mais cela ne se produit que lorsque je définis ma dernière notification avec un identifiant qui est un bigint (cela fonctionne très bien lorsqu’il est dans la plage int32)

Screen Shot 2023-02-15 at 4.55.36 PM
Screen Shot 2023-02-15 at 4.55.36 PM1454×154 31.6 KB

En examinant de plus près, je remarque que j’obtiens une réponse valide lorsque j’appelle http://localhost/notifications.json?limit=30 :

{
  "notifications": [
    {
      "id": 2148935910,
      "user_id": 2,
      "notification_type": 5,
      "read": true,
      "high_priority": false,
      "created_at": "2023-02-16T00:50:53.135Z",
      "post_number": 2,
      "topic_id": 8,
      "fancy_title": "Welcome to the Lounge",
      "slug": "welcome-to-the-lounge",
      "data": {
        "topic_title": "Welcome to the Lounge",
        "original_post_id": 13,
        "original_post_type": 1,
        "original_username": "blarouche",
        "revision_number": null,
        "display_username": "blarouche"
      }
    }
  ],
  "total_rows_notifications": 1,
  "seen_notification_id": 1001,
  "load_more_notifications": "/notifications?offset=60&username=bjlarouche"
}

Cependant, une fois que je passe le paramètre recent de la requête, c’est-à-dire http://localhost/notifications.json?recent=true&limit=30, et c’est ce qui est appelé par le menu de notification ?

{
  "errors": [
    "The requested URL or resource could not be found."
  ],
  "error_type": "not_found"
}

Screen Shot 2023-02-15 at 5.18.19 PM
Screen Shot 2023-02-15 at 5.18.19 PM2004×562 108 KB

edit : nous pensons que c’est parce que current_user.seen_notification_id est toujours un entier :thinking:

26

Je pense que nous sommes tirés d’affaire !

Nous avons constaté que nous devions migrer les colonnes dans d’autres tables qui référençaient notification_id en plus de notifications.id lui-même. Sinon, services/notifications.rb ou services/badge_granter.rb généreraient des erreurs.

Pour toute autre configuration de forum importante qui rencontrera ce problème à l’avenir avec les notifications, voici ce que nous avons fait…

Au total, nous avons dû migrer quatre colonnes dans quatre tables :

  1. notifications.id
  2. user.seen_notification_id
  3. user_badges.notification_id
  4. shelved_notifications.notification_id

Nous avons initialement procédé pour le point n°1 avec la commande ALTER suggérée ci-dessus, mais nous avons ensuite opté, comme mentionné, pour utiliser les migrations ActiveRecord, de sorte que les fichiers de migration s’ajoutent au schéma.

20230215070319_change_notifications_id_to_bigint.rb 
# frozen_string_literal: true

class ChangeNotificationsIdToBigint < ActiveRecord::Migration[6.1]
    def change
        change_column :notifications, :id, :bigint
    end
end
20230215070320_change_user_seen_notification_id_to_bigint.rb 
# frozen_string_literal: true

class ChangeUserSeenNotificationIdToBigint < ActiveRecord::Migration[6.1]
    def change
        change_column :users, :seen_notification_id, :bigint
    end
end
20230215070321_change_user_badges_notification_id_to_bigint.rb 
# frozen_string_literal: true

class ChangeUserBadgesNotificationIdToBigint < ActiveRecord::Migration[6.1]
    def change
        change_column :user_badges, :notification_id, :bigint
    end
end
20230215070322_change_shelved_notifications_notification_id_to_bigint.rb 
# frozen_string_literal: true

class ChangeShelvedNotificationsNotificationIdToBigint < ActiveRecord::Migration[6.1]
    def change
        change_column :shelved_notifications, :notification_id, :bigint
    end
end

Nous avons un Dockerfile personnalisé dans notre configuration (nous construisons des images afin de pouvoir exécuter discourse et sidekiq sur des ressources distinctes dans Kubernetes), donc copier ces fichiers dans /db/migrate dans le cadre de notre Dockerfile a été simple.

Ensuite, nous avons simplement laissé rake db:migrate s’occuper du reste. Une fois que nous avons effectué un redémarrage progressif de tous nos pods discourse et sidekiq, tout fonctionnait comme prévu :crossed_fingers:.

3 « J'aime »
27

Excellentes données, nous allons simplement prendre notre courage à deux mains et migrer cela dans le cœur de Discourse vers un bigint. C’est une opération coûteuse, mais cela se reproduira certainement sur les grands forums, autant donc le corriger.

Note… le sujet initial porte sur post_id… le dépassement de cette limite sera beaucoup plus difficile que pour les notifications. 2,1 milliards de messages vont certainement arriver, mais le coût de la conversion de post_id en bigint est bien plus élevé que celui de notification_id. Nous pouvons attendre un peu pour cette bombe à retardement.

4 « J'aime »
28

Brandon,

Je pense que nous allons finir par opter pour :

L’équilibre à trouver ici est que je ne veux pas « punir » 40 000 instances Discourse à cause des un ou deux forums hors normes qui ont réussi l’exploit incroyable de 2,5 milliards de notifications.

Qu’en penses-tu ?

(Remarque : cela rattrapera également au moins une chose que tu as manquée - il y a aussi une table de chat)

3 « J'aime »
29

Ce serait génial et cela ressemble à une solution intelligente :slight_smile:

33

Pour faire un retour sur ce sujet - ceci a maintenant été fusionné. :partying_face:

5 « J'aime »
34

Ce sujet a été automatiquement fermé après 4 jours. Les nouvelles réponses ne sont plus autorisées.