Feat/week04 #4

ekhvalov · 2023-11-11T16:48:56Z

Message Producing

Реализован services/outbox
Реализован services/msg-producer
Реализован services/outbox/jobs/send-client-message
Можно включить/выключить шифрование для публикующихся сообщений

Managers Load

Реализован services/manager-pool/in-mem
Реализован services/manager-load

Manager API

Генерация кода на базе контракта менеджерского API поддержана в task gen:api
Контракт менеджерского API можно получить через debug server
Контракт менеджерского API можно посмотреть в Swagger UI
Создание сервера унифицировано с помощью пакета internal/server
В Keycloak заведёны новые клиент chat-ui-manager, роль support-chat-manager и пользователь-менеджер с этой ролью
Реализован хендлер и юзкейс для /v1/getFreeHandsBtnAvailability, недостающие юнит-тесты написаны
Реализован хендлер и юзкейс для /v1/freeHands
/v1/freeHands возвращает ошибку с кодом 5000 в случае "перегрузки" менеджера
[Optional] /getFreeHandsBtnAvailability возвращает более детальный статус, чем true / false

Validation

Поля в конфигах и опциях валидируются, по возможности используются более специфичные теги, чем просто required

Testing

Новый код покрыт юнит-тестами, statements coverage > 90%
Client UI: Можно отправить сообщение и увидеть его через kcat
Manager UI: После нажатия на кнопку Ready to Problems она задизейблится и сохранит это состояние после обновлении страницы
Юнит-тесты проходят (task tests)
Интеграционные тесты проходят (task tests:integration)
E2E тесты проходят (task tests:e2e)

MysterySuperhero

Посмотрите, пожалуйста, комменты по репозиторию jobs и пулу менеджеров 🙂

MysterySuperhero · 2023-11-13T05:14:20Z

internal/repositories/jobs/api.go

+	var j *store.Job
+
+	findAndReserve := func(ctx context.Context) error {
+		tx := store.TxFromContext(ctx)


Важный момент, отталкиваясь от которого надо внести правки – репозиторий ничего не знает про транзакции. Он должен каким-то образом работать, как в режиме выполнения запроса в транзакции, так и в режиме выполнения запроса просто на соединении, и сам он этим управлять не может. Этим управляет юзкейс / сервис.
Могут возникнуть вопросы (хотя по коду ниже видно, что вы в целом понимаете суть, но на всякий случай ещё раз озвучим её). Абстрактный "как это сделать" и вполне конкретный вопрос "как нужно сделать в этом случае".

Как это сделать?

В третьем модуле вы как раз реализовали и использовали метод RunInTx() структуры Database, которая из недр пакета store. Обратите внимание, как там написан код и на что был сделан основной расчет. Теория была в шагах 1, 2 и в ссылках в них, поэтому пройдитесь, пожалуйста, по ним повторно на всякий случай.

Краткая выжимка:

Для прозрачной передачи транзакции между слоями используется контекст.

Транзакцию стартует юзкейс / сервис, вызывая метод RunInTx.

RunInTx кладет запущенную транзакцию в контекст и отдает контекст в функцию-коллбек, которую будет с этим контекстом вызывать (обратите внимание, что ещё этот метод делает).

Внутри функции-коллбека вызываем методы репозиториев, которые хотим выполнить транзакционно.

Репозитории уже в своих методах используют структуру Database, и вызывают у неё методы для выполнения запрсов. Например, Query().

Внутри Database.Query() есть вызов ещё одного метода loadClient, который как раз и решает где выполнять запрос: в транзакции или на соединении. И определяет он это как раз по наличию или отсутствию транзакции в контексте, как вы это сделали здесь.

P.S. можем в Телеграмах в канале / личке обсудить, если возникнут вопросы.

Как нужно сделать в этом случае?

Вспомним задачу: атомарно найти незанятую задачу и занять её. Атомарность требует выполнения этого всего либо одним запросом, либо в транзакции несколькими запросами.
Спойлер: мы это сделали одним запросом. Можете посмотреть в сторону CTE. Но и против выполнения в двух запросах не возражаем (возможно, потребуется правка заготовок и тестов).

Поэтому либо делаем одним запросом и тогда транзакции не нужны.
Либо уносим RunInTx на уровень сервиса и внутри вызываем методы репозитория, которые проделают нужные нам штуки.

Переписал с использованием двух запросов. Про CTE сходу не нашёл как сделать, по этому подсмотрю ваше решение.

Транзакцию стартует юзкейс / сервис, вызывая метод RunInTx.

Вы упустили этот момент 🙂 Т.к. старт RunInTx остался в репозитории.

Не будем вас больше мучать, посмотрите, пожалуйста, в авторское решение

MysterySuperhero · 2023-11-13T05:15:04Z

internal/repositories/jobs/api.go

+	findAndReserve := func(ctx context.Context) error {
+		tx := store.TxFromContext(ctx)
+		if nil == tx {
+			return fmt.Errorf("no transaction in context")


Если нет аргументов для форматирования, то используйте errors.New() 🙂

MysterySuperhero · 2023-11-13T05:17:08Z

internal/repositories/jobs/api.go

+
+		j, err = j.Update().AddAttempts(1).SetReservedUntil(until).Save(ctx)
+		if err != nil {
+			if isAttemptsExceededError(err) {


Это похоже на "логику", которой в репозитории не место. Вы и так возвращаете job-у с количеством попыток, поэтому на это количество попыток стоит смотреть в сервисе и в зависимости от него что-то делать 🙂

MysterySuperhero · 2023-11-13T05:24:20Z

internal/server-manager/v1/handlers_free_hands.go

+		case errors.Is(err, freehands.ErrInvalidRequest):
+			return internalerrors.NewServerError(http.StatusBadRequest, "invalid request", err)
+		case errors.Is(err, freehands.ErrManagerOverloaded):
+			return internalerrors.NewServerError(int(ErrorCodeManagerOverloadedError), "manager overloaded", err)


MysterySuperhero · 2023-11-13T05:27:01Z

internal/services/manager-pool/in-mem/service.go

+	return nil
+}
+
+func (s *Service) Get(_ context.Context) (types.UserID, error) {


nitpick: сплошное полотно текста тяжеловато читается. Делите, по возможности, отступами код на смысловые группы

Исправил

MysterySuperhero · 2023-11-13T05:34:56Z

internal/services/manager-pool/in-mem/service.go

+	s.mutex.RLock()
+	_, exists := s.store[managerID]
+	s.mutex.RUnlock()
+	if exists {


Кмк, здесь возможна гонка, в результате которой один менеджер окажется дважды в пуле.

Параллельно одновременно вызывается дважды этот метод с одним и тем же методом.
Параллельно дважды захватывается мьютекс на чтение. Оба раза убедились, что менеджера в мапе нет. Проскакиваем 61-ую строку. Дважды добавляем менеджера в мапу.

Кмк, здесь и в Get оптимизации с RLock излишни и приводят к усложнению кода. А профит какой от них непонятно

Исправил

MysterySuperhero · 2023-11-13T05:53:15Z

internal/services/outbox/service.go

+	s.rw.RLock()
+	_, exists := s.registry[job.Name()]
+	s.rw.RUnlock()
+	if exists {


Вроде как, аналогичная проблема может быть.

Но мы даже и не парились здесь насчет мьютексов, честно говоря

Исправил

MysterySuperhero · 2023-11-13T05:55:08Z

internal/services/outbox/service.go

+
+func idle(ctx context.Context, idleTime time.Duration) <-chan struct{} {
+	ch := make(chan struct{})
+	go func() {


А зачем стартовать отдельную горутину, чтобы потом все равно заблокироваться на канале? Кмк, просто селекта и таймера достаточно должно быть

Исправил

MysterySuperhero · 2023-11-13T05:56:53Z

internal/services/outbox/service.go

+}
+
+func (s *Service) handleJob(ctx context.Context, job jobsrepo.Job) error {
+	s.rw.RLock()


Тоже особого смысла не имеет, кмк, т.к. в рантайме не предполагается, что зарегистрированные джобы будут удаляться

Исправил

MysterySuperhero

🖖 🖖 🖖

ekhvalov added 8 commits November 4, 2023 15:20

STEP-4 Outbox service.

bca8df6

STEP-4 Kafka.

7ababf5

STEP-4 msg-producer, send-client-message, e2e tests.

55dbb82

STEP-4 manager-pool, manager-load.

d4dc541

STEP-4 freeHands API.

c34ade9

STEP-4 vendor

99a6025

STEP-4 missed file added.

319d3fc

STEP-4 another missed file added.

efaa699

MysterySuperhero reviewed Nov 13, 2023

View reviewed changes

Antonboom self-requested a review November 18, 2023 12:07

Antonboom assigned Antonboom and unassigned Antonboom Nov 18, 2023

Antonboom removed their request for review November 18, 2023 12:08

Antonboom assigned MysterySuperhero Nov 18, 2023

ekhvalov added 4 commits November 19, 2023 19:00

STEP-4 Repo FindAndReserveJob fixed.

facbafd

STEP-4 Manager pool fixed.

6b296ee

STEP-4 Outbox fixed.

28f7085

STEP-4 Code coverage added.

0526b38

MysterySuperhero approved these changes Nov 21, 2023

View reviewed changes

STEP-4 solution code used for outbox service.

f93d9e6

ekhvalov merged commit a1d9543 into main Nov 21, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/week04 #4

Feat/week04 #4

ekhvalov commented Nov 11, 2023 •

edited

Loading

MysterySuperhero left a comment

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 21, 2023

MysterySuperhero Nov 13, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 13, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 13, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero Nov 13, 2023

ekhvalov Nov 19, 2023

MysterySuperhero left a comment

Feat/week04 #4

Feat/week04 #4

Conversation

ekhvalov commented Nov 11, 2023 • edited Loading

Message Producing

Managers Load

Manager API

Validation

Testing

MysterySuperhero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Как это сделать?

Как нужно сделать в этом случае?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MysterySuperhero left a comment

Choose a reason for hiding this comment

ekhvalov commented Nov 11, 2023 •

edited

Loading