Optimización de claimUserTickets: Mejora de rendimiento y refactorización #273
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
TextC0de
commented
Sep 18, 2024
| ticketIds, | ||
| }, | ||
| }); | ||
| const [events, tickets] = await Promise.all([ |
There was a problem hiding this comment.
Se paralelizaron las consultas de eventos y tickets usando Promise.all.
| userId: USER.id, | ||
| logger, | ||
| }), | ||
| trx.query.ticketsSchema.findMany({ |
There was a problem hiding this comment.
Se eliminó el bucle for que iteraba sobre cada ticket. Ahora procesamos los tickets en lote, lo que reduce el número de consultas a la base de datos y mejora el rendimiento.
| .execute(); | ||
|
|
||
| // Bulk query for existing ticket counts | ||
| const finalTicketsCount = await trx |
There was a problem hiding this comment.
Reemplazamos múltiples consultas individuales por una única consulta en lote para obtener el recuento de tickets. Esto reduce la carga en la base de datos y mejora el tiempo de respuesta.
| // If the ticket template has a quantity field, means there's a | ||
| // limit to the amount of tickets that can be created. So we check | ||
| // if we have enough tickets to fulfill the purchase order. | ||
| if (ticketTemplate.quantity) { |
There was a problem hiding this comment.
Se eliminó este check inicial ya que estamos haciendo un check final en el recuento de los userTickets existentes.
Esto mejora el rendimiento para el caso de uso más común de éxito en el que no se rebasa la cantidad máxima de stock
TextC0de
commented
Sep 19, 2024
| userTicketsSchema.ticketTemplateId, | ||
| ticketTemplates.map((t) => t.id), | ||
| ), | ||
| inArray(userTicketsSchema.approvalStatus, [ |
There was a problem hiding this comment.
Aquí me basé en el código anterior en el que se tomaban en cuenta solo estos dos estados, approved y pending, pero tal vez deberíamos tambien tomar en cuenta a:
- Gifted
- Gifted accepted
- Not required
¿Que piensan?
TextC0de
commented
Sep 29, 2024
Signed-off-by: Ignacio Guzmán <[email protected]>
TextC0de
added a commit
that referenced
this pull request
Oct 15, 2024
Este PR implementa la funcionalidad de regalo de tickets (relacionado a #30, #31) y además incorpora optimizaciones de rendimiento propuestas en el PR #273. Se incluyen los siguientes cambios principales: 1. Nuevas tablas y relaciones: - `user_ticket_gifts`: Almacena información sobre los regalos de tickets. - Actualización de `user_tickets` con nuevos estados y relaciones. 2. Endpoints GraphQL: - Mutación `giftMyTicketToUser`: Permite a un usuario regalar su ticket. - Mutación `acceptGiftedTicket`: Permite al receptor aceptar un ticket regalado. - Query `myTicketGifts`: Obtiene los regalos de tickets enviados o recibidos por el usuario actual. 3. Lógica de negocio: - Implementación de validaciones para evitar auto-regalos y exceder límites de tickets. - Manejo de estados de regalo (pendiente, aceptado, rechazado, cancelado, expirado). - Cálculo de fechas de expiración para regalos. 4. Servicio de correo electrónico (solo para 9punto5, luego hay que extenderlo): - Nuevas plantillas de correo para notificaciones de regalo de tickets. - Implementación de métodos para enviar correos de confirmación de regalo y aceptación. 5. Actualizaciones en flujos existentes: - Modificación de `claimUserTicket` para manejar regalos de tickets durante la compra. 6. Pruebas: - Nuevos tests para cubrir los escenarios de regalo de tickets. ## Optimizaciones de rendimiento (del PR #273): Dado que estamos añadiendo más complejidad a la mutación `claim`, se han incorporado las optimizaciones propuestas en el PR #273. Estas incluyen: - Eliminación de verificaciones redundantes. - Paralelización de operaciones usando `Promise.all`. - Optimización de consultas a la base de datos, reemplazando múltiples consultas individuales por consultas en lote más eficientes. Estas optimizaciones, aunque no cruciales, son relevantes para mantener el rendimiento de la mutación `claim` a medida que se añade nueva funcionalidad. Cualquier feedback o sugerencia para mejorar la implementación es bienvenido.
|
Se implementó en #286 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Este PR optimiza la función
claimUserTicket, mejorando su rendimiento y simplificando su lógica.Los cambios principales incluyen:
Eliminación de verificaciones redundantes:
Paralelización de operaciones:
Optimización de consultas a la base de datos:
Detalle pruebas de rendimiento
Se realizaron 15 iteraciones de los tests encontrados en src/schema/userTickets/tests/claimUserTicket
Se hizo un análisis del rendimiento de los tests y estos fueron los resultados.
Versión main (actual en producción):
Nueva versión (textcode/improve-purchase-order-performance):
Mejora de rendimiento: 1.02%
Resumen de pruebas de rendimiento
La nueva versión muestra una mejora ligera pero consistente en el rendimiento.
Aunque la diferencia es pequeña en los tests actuales, es importante notar que:
Promise.ally la reducción de consultas a la base de datos, deberían proporcionar beneficios más significativos en entornos de producción con mayor carga.Cambios en los tests: