Erro: inteiro fora do intervalo

Bug
22

Isso resolveu o problema para post_actions, obrigado pela sugestão!

Estamos vendo principalmente duas falhas ainda:

  1. GET https://devforum.roblox.com/notifications está retornando 404 para usuários que têm notificações com IDs bigInt.

  2. O job agendado Jobs::GrantAnniversaryBadges falha.

É possível que um modelo/controlador/serviço de notificações ainda esteja codificado para definir o campo notifications.id como um inteiro?

23

muito improvável … 404 não parece certo, há algo em /logs que descreve o problema?

24

Infelizmente não consigo encontrar nada particularmente útil em /logs, mas quando reproduzo isso localmente, vejo a requisição chegar e depois 404, mas isso só acontece quando defino minha última notificação com um ID que é um bigint (funciona bem quando está dentro do intervalo 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

Investigando, percebo que recebo uma resposta válida ao chamar 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"
}

No entanto, assim que passo o parâmetro recent, ou seja, http://localhost/notifications.json?recent=true&limit=30, e é isso que o menu de notificações chama?

{
  "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: estamos pensando que é porque o current_user.seen_notification_id ainda é um inteiro :thinking:

26

Acho que agora estamos livres!

Percebemos que precisávamos migrar colunas em outras tabelas que referenciavam notification_id além de notifications.id. Caso contrário, services/notifications.rb ou services/badge_granter.rb lançariam erros.

Para qualquer outra configuração de grande fórum que encontrar isso no futuro com notificações, aqui está o que fizemos…

No total, tivemos que migrar quatro colunas em quatro tabelas:

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

Inicialmente, fizemos o item nº 1 com o comando ALTER sugerido acima, mas depois, como mencionado, optamos por usar migrações ActiveRecord, para que os arquivos de migração se somem ao esquema.

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

Temos um Dockerfile personalizado em nossa configuração (construímos imagens para que possamos executar discourse e sidekiq em recursos separados no Kubernetes), então fazer com que esses arquivos sejam copiados para /db/migrate como parte de nosso Dockerfile foi simples.

Em seguida, apenas deixamos rake db:migrate cuidar do resto. Assim que fizemos uma reinicialização gradual de todos os nossos pods de discourse e sidekiq, tudo estava funcionando como esperado :crossed_fingers:.

3 curtidas
27

Excelente dado, vamos simplesmente aceitar e migrar isso no core discourse para um bigint. É um movimento caro, mas certamente isso voltará a aparecer em fóruns grandes, então é melhor corrigirmos.

Nota… o OP é sobre post_id… estourar isso será muito mais difícil do que notifications. 2,1 bilhões de posts certamente acontecerão, mas o custo de transformar post_id em bigint é muito mais caro do que notification id. Podemos esperar um pouco por esse time bomb.

4 curtidas
28

Brandon,

Acho que podemos acabar optando por:

O equilíbrio aqui é que não quero “punir” 40 mil instâncias do Discourse por causa de um ou dois fóruns atípicos que conseguiram atingir o feito incrível de 2,5 bilhões de notificações.

O que você acha?

(Observação: isso também pegará pelo menos um que você perdeu - há uma tabela de chat também)

3 curtidas
29

Isso seria ótimo e parece uma solução inteligente :slight_smile:

33

Para voltar aqui - isso agora foi mesclado. :partying_face:

5 curtidas
34

Este tópico foi automaticamente fechado após 4 dias. Novas respostas não são mais permitidas.