-
Notifications
You must be signed in to change notification settings - Fork 371
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
Adiciona coluna Score ao Content e o procedimento de cálculo do Score #702
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Caso alguém tente usar a Preview não vai funcionar, pois só depois de ouvidas as opiniões e aprovadas as mudanças, vamos precisar rodar a |
Ahhh que massa!!!! Eu vou precisar sair mas em alguns minutos volto porque quero ler esse PR com carinho, mas olhando por cima uma sugestão é mover a função que está em stored procedure para uma migration de fato. Eu não fiz isso nas funções do firewall e me arrependi (tanto que mais para frente quero pagar por esse erro e migrar elas). Digo isso, porque dá para colocar a criação da função dentro de uma migration: tabnews.com.br/infra/migrations/1655868441752_create-table-balance-operations.js Lines 47 to 83 in b51a110
E se precisar sobrescrever algo para os testes, sem problemas, basta dar o replace. Assim você ganha de graça rodar ela por toda infraestrutura que já temos de migrations. E muito massaaa que essa feature ta vindooooo 😍 |
Minha primeira escolha foi essa e não funcionou... Mas vou ver depois com mais calma qual foi o motivo No commit c2fd24e eu acabei criando a coluna tabcoins no contents, pois o esforço para calcular o score e o saldo de tabcoins é o mesmo, então não tem porque salvar só o score. Depois que rodar o script para popular essa coluna para o conteúdos antigos dá pra usar isso no lugar de recalcular toda vez que busca os conteúdos |
Dessa forma não funciona o UPDATE |
Tava pensando aqui que isso possa mostrar o valor errado de tabcoins se não considerarmos tudo que pode modificar esse valor, por exemplo, na branch #699 onde num ato de "nuke" é desfeito todas as operações do usuário (ao criar balance_operations zerando as operações). O valor correto só seria mostrado num novo evento de up/down vote. Talvez o que devemos fazer é a estratégia adotada no Pagar.me entre a A estratégia da Não sugiro fazer a Faz sentido voltar a calcular em tempo-real esse valor? Até porque no final das contas, a gente em algum momento deve fazer o cache do resultado da API na borda. |
Não sei se fiz algo errado, mas fiz essa migration abaixo e os testes do exports.up = async (pgm) => {
await pgm.createFunction(
'get_balance_and_update_score',
[
{
name: 'score_type_input',
mode: 'IN',
type: 'text',
},
{
name: 'recipient_id_input',
mode: 'IN',
type: 'uuid',
},
],
{
returns: 'integer',
language: 'plpgsql',
replace: true,
},
`
DECLARE
total_balance integer;
positive_balance integer;
negative_balance integer;
new_score decimal;
BEGIN
positive_balance := (
SELECT
COALESCE(sum(amount), 0)
FROM
balance_operations
WHERE
balance_type = score_type_input
AND recipient_id = recipient_id_input
AND amount > 0
);
negative_balance := (
SELECT
COALESCE(sum(amount), 0)
FROM
balance_operations
WHERE
balance_type = score_type_input
AND recipient_id = recipient_id_input
AND amount < 0
);
new_score := COALESCE(trunc((positive_balance + 0.9208) / (positive_balance - negative_balance + 2.8416),3), 0.5);
total_balance := COALESCE(positive_balance + negative_balance, 0);
UPDATE contents
SET
score = new_score,
tabcoins = total_balance
WHERE
id = recipient_id_input;
RETURN total_balance;
END;
`
);
};
exports.down = false; |
Esse cache só é utilizado para a UI, não para transações e nem ranqueamento. Acho que não faz sentido voltar a calcular isso a cada busca dos conteúdos só para mostrar na UI, já que o valor só muda quando ocorre um voto. E a atualização dele ocorre dentro da transação de voto, então qual é a chance dele ficar inconsistente? Sobre o retorno do valor que existia antes do conteúdo ser deletado, não vejo qual seria o problema pelo mesmo motivo, de que esse valor não é fonte da verdade para nenhuma transação. Mas se tem algum problema que não estou enxergando, melhor zerar esse campo ( Line 29 in b722182
O que pode mudar para
Vai saber que meleca eu tinha feito então, mas que show!!! |
O que destaquei ali em negrito que é o ponto, hoje nessa branch aqui, esse valor de fato só muda quando ocorre um voto e não vejo probabilidade alguma de se tornar inconsistente 🤝 Mas a partir do PR #699 vai ser inserido um segundo caso em que esse valor é alterado fora de um voto... e mais para frente outros casos irão surgir de outras features que poderão influenciar esse valor. É por conta da evolução de um software em que as coisas vão cada vez mais se atravessando que frases como essa são ditas.
Se performance é um problema hoje e precisamos fazer cache, sugiro primeiro fazer no retorno da API com |
@aprendendofelipe eu estava testando em uma outra branch localmente aqui e posso ter passado uma informação errada sobre a migration, porque a função estava sendo criada também pelo |
Acho que entendo sua preocupação com relação ao cache dos tabcoins, pois tenho a mesma preocupação, só que estou mirando um pouco para o lado 👀, ou seja, para o score. Tanto que isso para mim é mais um motivo pra manter juntos os cálculos de score (para o rank) e dos tabcoins (para a UI). O ponto é que precisamos recalcular o score sempre que o balanço de tabcoins mudar. Não tem como fugir disso para termos um rank sempre atualizado. E daí a decisão fica entre:
Sim, temos a opção de não confiar e recomputar tudo sempre, mas não adianta fazer isso para o saldo de tabcoins e deixar de lado o score. E o score sozinho pode passar despercebido, até tenho impressão que isso está acontecendo nessa conversa. É o score que vai ranquear os conteúdos, então é ele que não pode ficar inconsistente. E se a gente não salvar um cache para o score, aí a complexidade para efetuar o ranqueamento entra em outro nível. E se vamos salvar o score, não há razão para não salvar o saldo de tabcoins também. A questão de performance é só um bônus, não é o ponto principal. O principal mesmo é... que nome daremos pra essa função? haha Resumindo... O foco desse PR é garantir que o score esteja sempre atualizado 🤝 P.S. Mesmo performance não parecendo ser problema por enquanto, executar essas duas funções uma na sequência da outra é desperdício: tabnews.com.br/models/balance.js Lines 60 to 61 in 97c625b
Eu tinha deixado assim na primeira versão, mas logo percebi que elas fazem as mesmas consultas ao banco e isso que me inspirou a já salvar o cache tanto do score quanto de tabcoins, mas mesmo sem salvar o cache de tabcoins, nada impede retornar o saldo na mesma função. |
Por isso que essa conversa está sendo tão importante 😍 Sobre a inconsistência, talvez essa seja a minha angústia sobre salvar qualquer tipo de dado na verdade e acredito que o Mas essa frase abaixo me deu uma ideia:
Será que ao invés de ficar caçando no código por pontos onde a camada da aplicação precise ficar atualizando o |
E agora que percebi um ponto cego quando falo sobre o PR #699 que o Dado a isso, o que acha de usar triggers? |
Boa! Dado que a Reflexões relacionadas:
|
Concordo 🤝
Justo! Eu antes gostaria de entender com o pessoal do Pagar.me porque eles não usaram a estratégia de Triggers para o saldo deles. Se rolar algum call com eles eu te puxo pra dentro 🤝 |
Esse commit 77ff211 é uma prova de conceito do novo ranqueamento que atinge páginas além da primeira. Por enquanto ele está utilizando o saldo da Tabcoins, que estão sendo computados em tempo real para todos os conteúdos, pois o peso desse cálculo será muito parecido com o de computar o score. Então a ideia é utilizar o score, seja computado em tempo real (se não pesar demais) ou utilizando um valor em cache. |
24195e3
to
96ac9a7
Compare
Interessante! Pegando pelo comentário ali em cima:
Dado que retornou |
A questão é: Por que estava sendo executado um teste que não existia nessa branch? 😅 Não existia esse teste, pois a branch de origem era mais antiga do que a que você implementou o nuke. Dado que o teste estava sendo executado, sabe-se lá a razão, então é normal dar o erro, pois ele precisava ser adequado. Dava erro nas transações de votos porque não existia a função |
@filipedeschamps, pensando na sua dúvida, acho que talvez nessa conversa não estamos separando de maneira clara duas situações diferentes que envolvem saldos de TabCoins.
Pode até parecer mais simples tratar os dois casos da mesma maneira, mas os requisitos deles são diferentes, então tratar de maneira igual complica o ranqueamento. Apenas como comparação, vou começar pelo saldo dos usuários. Em nenhuma situação precisamos computar o saldo de diversos usuários. Hoje nós computamos de um único usuário sempre que chamamos o endpoint Já para o saldo dos conteúdos, ele tem duas funções, ranquear por relevância e informar ao usuário como o conteúdo está avaliado até o momento. E a questão do ranqueamento é que é o ponto que nos impede de lidar com esses saldos da mesma maneira que o saldo dos usuários. Pois, para ranquear, precisamos do saldo de diversos conteúdos. Quantos saldos precisamos exatamente é algo que irá depender da estratégia de ranqueamento que vamos adotar. No mínimo 30, que é o que ocorre hoje, onde filtramos primeiro por tempo, percorrendo de 30 em 30, e daí só precisamos calcular desses 30, mas é uma situação bem longe da ideal. No lado oposto temos o "ideal" (opinião minha pelos quesitos qualidade e velocidade do ranqueamento e facilidade de implementação), que seria já ter salvos e indexados todos os saldos dos conteúdos e indexado também pelo tempo, para aí sim ter liberdade total para ranquear de forma rápida qualquer quantidade de conteúdos. As estratégias que eu pensei aqui, e que tentam fazer algo no meio entre esses dois extremos, ocorre que vai melhorando a qualidade do resultado do ranqueamento ao mesmo tempo que aumenta o tempo necessário para efetuar esse ranqueamento. E pelo que entendi a v2 do Pagar.me ajudaria a diminuir esse tempo, mas longe de ser comparável ao caso de saldo já indexado. Então se quisermos melhorar a qualidade, precisamos diminuir a frequência com que ocorre o ranqueamento para algo que o banco consiga dar conta. E diminuir a frequência do ranqueamento significa ter dados desatualizados por mais tempo. Tudo bem, é aceitável termos os dados desatualizados por um certo período e equilibrar esse período com o nível de qualidade do ranqueamento. Mas a questão que fica é... Por que ir por esse caminho intermediário se a situação "ideal" é mais simples de implementar e traz os melhores resultados? Salvando no banco os saldos já calculados, nós nunca precisaremos recalcular tudo. Só é preciso recalcular o conteúdo que receber qualquer qualificação. E para esse cálculo temos duas opções:
A alternativa 1 é simples de implementar e tudo fica sempre estar atualizado em tempo real. O único risco que vejo nela é alguma implementação futura que esqueça de atualizar o cache quando o saldo mudar. É um risco pequeno e afetaria o ranqueamento desse único conteúdo, mas não podemos ignorar esse risco. Então a alternativa 2 pode ser adotada, ou como única solução (caso aceite um período desatualizado), ou como contingência para caso a primeira falhe. Ela pode verificar de tempos em tempos e atualizar se encontrar algo inconsistente, ao mesmo tempo disparando um alerta que nos avise que existe algum furo na lógica 1. Cadê a Turma nos ajudando a pensar nisso? 😅 |
O que acho que poderiam ser os próximos pequenos passos possíveis que podemos dar para chegar na solução que eu estou propondo:
Todos são passos simples, mas que no fim fazem uma grande diferença. E tudo isso pode ir aos poucos para produção, mesmo que já tenhamos testado tudo em homologação. |
Só estou dando um E eu quero fazer um dump do banco de produção, deixar anônimas as informações sensíveis e te passar para facilitar lapidar o algoritmo. Se você ou alguém souber de algum script/rotina que facilite deixar os dados anônimos, por favor não deixa de me informar. O dump do banco lá do RDS já fiz no passado na época do preenchimento do histórico das TabCoins, então essa parte está ok 👍 só falta fazer um update em massa nos dados sensíveis respeitando as constraints. |
@aprendendofelipe para ficar mais fácil testar qualquer tipo de algoritmo, fiz o dump do banco de produção e deixei os dados anônimos, usei a seguinte query: -- 1) ANONYMIZE EMAIL, PASSWORD AND NOTIFICATIONS
WITH
random_values AS (SELECT id, floor(random() * 9999999999) AS value FROM users)
UPDATE
users
SET
email = (select 'anonymized-' || random_values.value || '@localhost.com'),
password = 'anonymized',
notifications = TRUE
FROM
random_values
WHERE
random_values.id = users.id;
-- 2) DESTROY THE FOLLOWING TABLES:
TRUNCATE sessions;
TRUNCATE activate_account_tokens;
TRUNCATE email_confirmation_tokens;
TRUNCATE reset_password_tokens;
-- 3) REMOVE ALL 'draft' AND 'deleted' CONTENTS
DELETE FROM contents WHERE status in('draft','deleted');
-- 4) ANONYMIZE 'events' IPs
UPDATE events SET originator_ip = '127.0.0.1'; O que acha? Fiz |
Estamos com uma
Acho que é isso! Bora! |
DisclaimerTudo o que eu escrever abaixo tem em mente o estágio atual do projeto que é estar otimizado para contribuição e, já antecipando último item Conclusão, são os passos que me deixarão mais confortável e seguro com o que for feito daqui para frente. E com isso poderá haver coisas que tecnicamente não vão fazer sentido para um estágio futuro do projeto, mas há várias "escolhas invisíveis" que faço que eu mesmo não consigo mais perceber, mas que foi sendo compilado dentro da minha intuição pela experiência em projetos open source no passado. E também ao escrever todo esse texto, eu notei uma agústia que eu não estava conseguindo colocar o dedo em cima, mas que agora ficou claro, que é eu considerar que existe uma diferença enorme entre dados inconsistentes e dados desatualizados... vamos ver se o que eu escrevi vai fazer algum sentido. Bom, é bastante texto mas tudo foi escrito de coração ❤️ Disclaimer feito, essa é a resposta 😂Vou começar destacando um ponto, onde o contexto era sobre o cache para todos os tipos de balance:
Entendo total essa frase e eu de fato estou forçando calcular tudo da mesma forma 😅 🤝 estou pensando assim por consistência de implementação, consistência de dados e na modelagem da minha cabeça, todo tipo de saldo é a mesma coisa. Mas entendo que isso impacta na performance quando comparado a um valor calculado e indexado, meu único medo é estar distorcendo uma modelagem em favor de performance quando esse nível de performance não é necessário. Não estou afirmando que este é o caso, mas precisamos ver quais são os números de verdade. Eu bato nessa tecla porque em todo meu histórico em outros projetos open source, sempre valeu mais a pena manter as implementações previsíveis/uniformes do que "performáticas" (entre aspas para destacar que isso não significa devagar ao ponto de ser inutilizável), o que me leva nessa frase aqui:
Eu entendo o termo "facilidade de implementação" e concordo que é mais fácil quando estamos olhando apenas para o objetivo atual 🤝 mas não sei se eu concordo se especularmos sobre o futuro do projeto, pois as inconsistências na implementação e dados vão cada vez mais aumentando, precisando cada vez mais criar rotinas que compensem e isso já está acontecendo na estratégia de cozinhar as TabCoins, por exemplo aqui:
Não mais (como você vai apontar ali em baixo), o saldo das TabCoins do conteúdo podem ser influenciados por um Nuke. Então se alguém "construiu saldo em uma conta maligna" e negativou de forma artificial uma publicação e essa conta tomar um Nuke, tanto as TabCoins dessa publicação quanto o Score ficarão errados. Como efeito colateral para compensar isso, você sugeriu implementar uma outra rotina:
Não sou contra rotinas, inclusive inevitavelmente a gente vai precisar delas para cálculos que não vão poder ser entregues em tempo-real, mas precisar implementar isso como efeito colateral de uma inconsistência causada pelo foco em performance no estágio atual do projeto não sei se é um princípio bom de seguirmos, pois seguir isso vai trazer cada vez mais complexidade, o que me leva para essa afirmação:
Justo, como destaquei antes, esse risco já é atual (Nuke) e há também uma probabilidade considerável de existir outras features que usem esses valores de formas que a gente não imagine e que tenha uma relação muito próxima com outros saldos, como o saldo do usuário. Agora voltando sobre o tópico performance, quando você sugeriu a rotina "De tempos em tempos verificar a tabela de balance e atualizar os dados dos conteúdos que tiveram qualificações nesse período" eu não consegui entender o que é "nesse período" exatamente. Será por exemplo um período fixo de "x tempo para trás" ou "todos os novos dados desde a movimentação X"? Se for a segunda alternativa, isso é o princípio do cache da SugestãoEntão minha sugestão de passos que vão me deixar tranquilos ao analisar o escopo total do projeto é o seguinte:
1. Calcular tudo em tempo-realNessa estratégia, a única coisa que sugiro mudar é a query que pega e ordena os dados (usando o seu algoritmo de janelas), sem mexer em mais nada, nem índices, stored procedures, nada. Talvez eu passaria essa lógica para um novo model Mas para essa sugestão conseguir parar em pé, precisamos especificar o que é um conteúdo Relevante na estratégia Como antecipado por email, reduzir o escopo dos dados para 1 semana (ou até 4 dias) muda drasticamente a performance dos resultados usando o seu algoritmo, vou deixar registrado aqui usando dados de Produção: Classificar todos os conteúdosClassificar 1 semana de conteúdo com seu algoritmoIsso considerando que foram retornados 1 semana de conteúdo sem algoritmoPor curiosidade, eu rodei a query que constrói a estratégia atual Então há uma penalização na performance da query em sí, mas nada que inviabilize o resultado, muito pelo contrário, pois trará uma resultado com uma qualidade maior com o seu algoritmo. 2. Criação de índices (só quando precisar)Depois da lógica feita no item anterior, a sugestão de fazer uma bateria separada de implementação de índices é para termos certeza que ao comparar o antes e depois teremos resultados melhores. Se numa paulada só mudarmos toda a lógica e já implementarmos o índice não teremos certeza de qual avanço isso teve, e se não tiver, é um problema, pois fazer índice não sai de graça. Ao fazer cada passo de forma separada poderemos mensurar o que cada parte teve de impacto (exatamente qual impacto), como estava sendo destacado nas publicações de melhoria, como nessa daqui nos itens 3. Cache na CDN (só quando precisar)Esse é o estágio que pode mais fazer diferença para os usuários que estão abrindo a Home (ou não, na verdade, só os que consomem a API como vou detalhar mais abaixo), com o trade-off de que essa implementação pode trazer dados desatualizados, mas que na minha visão é diferente de dados inconsistentes. Aqui podemos assumir várias estratégias de quem irá atualizar esse cache e eu começaria pela mais simples que é com um Mas fazer esse cache também não é simples, pois há uma chance consideravel que o 4. Cache
|
Show @filipedeschamps! 🤩 Muda toda a história se as páginas de "Relevantes" forem mostrar só os conteúdos bem recentes, pois aí só precisamos do Score de uma pequena parte da tabela, o que viabiliza manter o cálculo em tempo real. A performance deve ficar longe do ideal, mas, se ficar aceitável, podemos deixar o cache pra depois. Sobre os índices, cache e diff, é isso aí, perfeito! Então vou partir da sugestão de restringir por 1 semana e vamos ajustando 🚀 Apenas esclarecendo a questão do Nuke vs Cache direto no banco... Acho que não teria como fugir de atualizar o cache de todos os conteúdos afetados pelo Nuke, então isso teria que ser acionado automaticamente ao banir alguém. Mas pensaremos melhor sobre isso no momento de implementar esse cache. Para pensar depois e, se for o caso, implementar em outro momento: Restringir os relevantes por quantidade ao invés de tempo. Então iria existir uma quantidade fixa de páginas de conteúdos relevantes. Talvez 2 ou 3 páginas, o que daria 60 ou 90 conteúdos. Dependendo da performance ou se já existir cache, esse número pode ser maior. Independentemente do cache na CDN, acho que podemos retirar esse request adicional ( |
Showwww 😍
Faz total sentido!
Perfeito, também faz sentido 🤝 |
Esse PR prepara o sistema para trabalharmos com o score salvo na tabela
contents
do banco de dados.Por enquanto nada muda no ranqueamento, apenas vamos começar a popular a coluna
score
a cada novo conteúdo postado e a cada voto, mesmo que nos conteúdos antigos.migration
para adicionar a colunascore
.stored-procedure
que calcula o novoscore
sempre que o conteúdo recebe um voto.score
sempre recebe valores entre 0 e 1, onde 0 seria um conteúdo nada relevante e o 1 seria o totalmente relevante.stored-procedures
, foram criadas subpastas da pastastored-procedures
.buildOrderByClause
para receber umArray
deStrings
.score = 0.5
enquanto não receber votos, ou seja, está exatamente no meio caminho da relevância (no futuro isso pode mudar para já considerar a relevância e experiência do autor).A próxima etapa será executar um script para calcular o
score
de todos os conteúdos já postados.Após esse script, será possível seguir para o passo seguinte que será utilizar esse
score
para ranquear a lista de conteúdos.[edit]
8. Criados índices com o
score
ecreated_at
, pois serão utilizados para o ranqueamento da lista de conteúdos.9. Criada a coluna
tabcoins
na tabelacontents
pois ao calcular oscore
automaticamente obtemos o saldo de tabcoins, então não tem porque ficar recalculando isso a todo momento.[edit 2]
10. Corrigido o índice, pois a classificação por tempo é via
published_at
e nãocreated_at
.11. Como teste para a alternativa de calcular o Score em tempo real ao invés de calcular apenas quando houverem transações, a classificação por relevância está calculando o saldo de TabCoins em tempo real de todos os conteúdos root. Esse teste é válido, pois o peso de calcular o saldo de TabCoins e do Score são parecidos