Skip to content

Code review - #3

Open
Liudmi1a wants to merge 5 commits into
mainfrom
code-review
Open

Liudmi1a wants to merge 5 commits into
mainfrom
code-review

Conversation

@Liudmi1a

Copy link
Copy Markdown
Owner

No description provided.

@vladefr97 vladefr97 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не очень понял что в данном pull request реализовано? Просто какие-то пустые классы

Comment thread .gitignore
@@ -2,4 +2,5 @@ venv/
__pycache__/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нужно добавить в проект:

  • Readme.md c описанием структуры и функциональности проекта, списком команд для работы с проектом (запуск, настройка и тд)
  • env.example со списком необходимых .env переменных
  • docker-compose с необходимым окружением для запуска проекта (как минимум redis). В идеале и сам сервис запаковать в Dockerfile и добавить в docker-compose, чтобы весь проект можно было поднять одной командой, но достаточно хотя бы окружение

Comment thread rate_limit.py
global_key = f"rate_limit:global:{resource}:{current_window}"
ip_key = f"rate_limit:ip:{resource}:{client_ip}:{current_window}"
ttl = window_seconds + 10
lua_script = """

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем lua скрипт используете? Можно просто redis клиента использовать

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

В отчете я аргументировала выбор такого решения следующим образом.
"Redis выполняет Lua-скрипты атомарно — во время выполнения скрипта сервер не обрабатывает другие команды. Это исключает race condition между проверкой лимитов и увеличением счетчиков, что критически важно для корректной работы системы ограничения трафика при высокой нагрузке".
В более ранней версии я делала через пайплайны Redis (отдельно брала счётчики, проверяла и потом их увеличивала). Но при таком подходе возникала проблема. Если два запроса приходят в одно и то же время, они могут оба увидеть одинаковое значение счётчика, оба решить что лимит не превышен, и оба его увеличить.
Поэтому я переделала на Lua-скрипт. Он выполняется прямо в Redis и делает операции проверки и увеличения как одно целое. Пока скрипт работает, другие запросы ждут. Это полностью убирает race condition и гарантирует, что лимит никогда не будет превышен.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants