Инкременты №16-18 #16
Conversation
There was a problem hiding this comment.
Привет! Ты отлично поработал над кодом и над документацией API. Однако есть несколько моментов, которые стоит улучшить:
- Улучши именование некоторых методов и структур для большей ясности
- раздели компоненты для улучшения архитектуры
В целом, ты на правильном пути! Продолжай в том же духе
| var _ StorageService = Service{} | ||
|
|
||
| // Service struct, containing storage | ||
| type Service struct { |
There was a problem hiding this comment.
Сервис хранения слишком тесно связан с конкретной реализацией хранилища, попробуй разделить
There was a problem hiding this comment.
Здесь не догнал, как именно связан? Хранилище имеет интерфейсный тип, сервис ничего не знает вроде как о конкретном типе хранилища. Или я до конца не понял?
| // @Tags Metrics | ||
| // @Router /updates [post] | ||
| // @Summary Push list of metrics data as JSON | ||
| // @ID metrics_json_update_list |
There was a problem hiding this comment.
Назови проще UpdateMetricsBatch
| @@ -16,6 +16,7 @@ | |||
|
|
|||
| var _ MetricsStorage = DatabaseStorage{} | |||
|
|
|||
| // DB Storage | |||
| @@ -25,6 +27,7 @@ func (r Retrier) Run() error { | |||
| ) | |||
| } | |||
|
|
|||
| // Retrier constructor | |||
There was a problem hiding this comment.
Из фишек гоу, в структуре можно использовать функциональные опции для более гибкой настройки. Типа такого:
type RetryOption func(*Retrier)
func WithDelays(delays []time.Duration) RetryOption {
return func(r *Retrier) {
r.delays = delays
}
}
func NewRetrier(payloadFn func() error, retryIfFn func(error) bool, opts ...RetryOption) *Retrier {
r := &Retrier{
payloadFn: payloadFn,
retryIfFn: retryIfFn,
}
for _, opt := range opts {
opt(r)
}
return r
}Это позволит более гибко настраивать Retrier при создании.
| @@ -58,6 +69,18 @@ | |||
There was a problem hiding this comment.
Отличная работа по добавлению документации для Swagger 💪
| } | ||
|
|
||
| // Singleton | ||
| func GetProfiler() *Profiler { |
There was a problem hiding this comment.
Отличное решение использовать синглтон для профайлера 👍
| @@ -0,0 +1,54 @@ | |||
| //nolint:gocritic | |||
There was a problem hiding this comment.
Ну и тут хороший пример 😊
В этом pr:
errToStatusне очень удачное решение и его лучше переписать под явную обработку ошибок, но не стал это делать в рамках этого PR чтобы не забивать diffИзвиняюсь, что оформил PR в этот раз не по правилам. После гофермарта забыл про деление на ветки и отдельные PR :( Плюс, не думал что так разрастется из-за комментариев.
Тесты:
$ make unit-tests
Т.к. я пишу в vscode с включенным gopls, он автоматически форматирует весь код в соответствии со стандартом, так что форматирование через gofmt ничего не поменяло:
Сначала снимал профили просто в случайный момент времени, так:
Самыми большими нодами на графике были ноды gzip, поэтому решил попробовать заменить его на на https://github.com/klauspost/compress. Если я правильно понял, то снимок кучи выглядит по-разному в разные моменты времени, несмотря на то, что агент посылает запросы на сервер с интервалом в 1 секунду. Не всегда эти профили были похожи друг на друга (в graph и flamegraph). Но удалось поймать схожие снимки, и сравнивал их. Потом я все же решил что сравнение не совсем честное, и доработал профайлер таким образом, чтобы он вызывался в одни и те же моменты времени (один раз, на третий вызов BatchUpdateMetricsJSON). Затем сохранил их по очереди и сравнил, новый компрессор показывает отрицательные числа в утилите сравнения (
go tool pprof -top -diff_base=profiles/base.prof profiles/result.prof), т.е. потребляет меньше памяти.Также он показывает прирост по бенчмарку:
make build) и доступна через swagger на http://localhost:8080/swagger/index.html. В бою её, конечно, нужно генерировать отдельно и складывать куда-то в вики, например.