Comments 51
А какую проблему решает ваш шаблон проектирования?
На мой взгляд, вы забыли про базовые принципы, про простоту, про то, что явное лучше не явного. Все вот эти вот ваши паттерны проектирования оставьте там, откуда вы пришли в Go, они тут не нужны, не нужно пытаться «натягивать сову на глобус», ничего хорошего из этого не выйдет
А можно проблему описать словами, а не ссылкой "ту же"? Я не увидел, какое ваш шаблон имеет отношение к контексту, а значит кто-то из нас (либо я либо вы) плохо понимаем, что такое контекст и для чего он. Ну или, как вариант, я просто не понял вашей реализации ;))
Более менее просматривал код прокси. Первое, что пришло в голову — вы полностью выбираете body, а потом отдаёте бекенду? Ну… лучше бы так не делать.
Дальше про обработку ошибок — понимаю, что это proof of concept, да и код я смотрел с мобилки, но обработки таймаутов там нет, вариативности ошибок тоже… хотя может и лохо смотрел — вы можете сообщить о недоступности бекэнда или переключиться на живой бекэнд? Если на оба вопроса ответ "нет", то попробуйте добавить этот функционал, и, как мне кажется, тут-то и вылезут нюансы.
Более менее просматривал код прокси. Первое, что пришло в голову — вы полностью выбираете body, а потом отдаёте бекенду? Ну… лучше бы так не делать.
Ну почему же, прокси принимает данные и сразу же отправляет их на какой-либо backend сервер:
run := func(task job.Task) {
select {
case dd := <- p.conn.Downstream().RecvRaw():
p.conn.Upstream().Write() <- dd
p.conn.Upstream().WriteSync()
p.conn.Downstream().RecvRawSync()
task.Tick()
default:
task.Idle()
}
}Вот уже сам backend принимает данные в кадрах с помощью этой библиотеки: github.com/AgentCoop/net-dataframe — то бишь читает пока не получит полный кадр.
Дальше про обработку ошибок — понимаю, что это proof of concept, да и код я смотрел с мобилки, но обработки таймаутов там нет, вариативности ошибок тоже… хотя может и лохо смотрел — вы можете сообщить о недоступности бекэнда или переключиться на живой бекэнд? Если на оба вопроса ответ «нет», то попробуйте добавить этот функционал, и, как мне кажется, тут-то и вылезут нюансы.
Таймауты легко задавать с помощью метода
AddTaskWithIdleTimeout. Там есть пример в реализации timeout в 2 секунды для клиента:
pjob := job.NewJob(upstreamSrv.TcpAddr)
pjob.AddOneshotTask(proxyConn.ProxyConnectTask)
pjob.AddTask(p.conn.ProxyReadDownstreamTask)
pjob.AddTask(p.conn.ProxyReadUpstreamTask)
pjob.AddTask(p.conn.ProxyWriteDownstreamTask)
pjob.AddTask(p.conn.ProxyWriteUpstreamTask)
pjob.AddTaskWithIdleTimeout(p.downstream, time.Second * 2) // client connection timeout
pjob.AddTask(p.upstream)
<-pjob.RunInBackground()Отловить ошибки соединения тоже не составляет труда. Вот тут она вылезет и прервет выполнение Job.
conn, err := netMngr.reuseOrNewConn(p.upstreamServer.TcpAddr)
task.Assert(err)
а обработать ее можно вызовом метода:
go func() {
for {
select {
case <- balancerJob.JobDoneNotify():
_, err := balancerJob.GetInterruptedBy()
...
return
}
}
}()
пометив соответствующий сервер как недоступный.
А можно проблему описать словами, а не ссылкой «ту же»? Я не увидел, какое ваш шаблон имеет отношение к контексту, а значит кто-то из нас (либо я либо вы) плохо понимаем, что такое контекст и для чего он. Ну или, как вариант, я просто не понял вашей реализации ;))
Отвечу вопросом на вопрос: а как бы Вы решали задачу, если бы Вам нужно было реализовать прокси сервер? Наверняка же с помощью context.Context :) Потому что Вам нужны были бы сопроцедуры как для чтения/записи данных с клиента, так и для чтения/записи данных c upstream сервера и все это как-то нужно было оркестрировать. Или наверное я тогда действительно не понимаю для чего нужен context.Context, если бы я делал это не с его помощью.
Вы все время упоминаете context.Context, что он нужен для асинхронности или корутин. Но ведь это не так: контекст, как сообщает нам официальная документация — это переносчик состояния процесса или, что более специфично, текущего api вызова.
Чаще всего контекст используется для хранения в нем специфичной для запроса информации (например, проброса такой информации, как трейсинг), а также прокидывания сигналов отмены.
Т.е., с его помощью можно контролировать выполнение нижележащих вызовов, например задавать таймауты запросов.
Самое главное, что контекст понимают все встроенные в язык пакеты, связанные с какими-либо вызовами, а также все другие библиотеки.
Почему вы его противопоставляете своему подходу с джобами?
Package context defines the Context type, which carries deadlines, cancellation signals, and other request-scoped values across API boundaries and between processes.
Это все реализует Job только на более высоком уровне. Вызов метода Job.Cancel() таким же образом отменит выполнение своих задач. request-scoped values — это тот же Job value, задаваемое через конструктор job.NewJob() или непосредственно через устанавливающий метод .SetValue. К примеру в моей реализацией я переношу через него ссылку на сетевое соединение задачам, которые с ним работают.
Ну а прокидывать сигналы отмены не нужно, это все идет из коробки. Как я же упоминал ранее достаточно вызвать Job.Finish() или Job.Cancel() и все остальное сделает шаблон.
Вот пример использования WithCancel из официальной документации:
golang.org/pkg/context/#example_WithCancel
Тоже самое но уже с использованием Job:
play.golang.org/p/ge2FUO7a6WU
А теперь представьте как упрощается работа, когда у Вас не одна задача а 5 — 6 или даже 7 как это было в моем случае:
pjob.AddOneshotTask(proxyConn.ProxyConnectTask)
pjob.AddTask(p.conn.ProxyReadDownstreamTask)
pjob.AddTask(p.conn.ProxyReadUpstreamTask)
pjob.AddTask(p.conn.ProxyWriteDownstreamTask)
pjob.AddTask(p.conn.ProxyWriteUpstreamTask)
pjob.AddTaskWithIdleTimeout(p.downstream, time.Second * 2) // client connection timeout
pjob.AddTask(p.upstream)В случае с context.Context их нужно запустить, везде прокинуть context, чтобы иметь возможность их отменять и все это нужно делать руками.
В случае Job, вы просто пишете логику задач не беспокоясь о том как они будут запускаться и как отменяться. В некотором роде тут даже реализуются элементы контрактного программирования. К примеру в примере выше последние 6 методов ожидают установленное соединение в Job value — это клиенты, а задача proxyConn.ProxyConnectTask поставщик этого соединения выполняющая это предусловие. Пока это условие не будет выполнено — эти задачи не запустятся.
В самим задачах критические ошибки вы обрабатываете простыми вызовами task.Assert. Все, больше не о чем беспокоится — Вы сосредотачиваетесь на самой бизнес логике. Как только условия не соблюдено — задачи останавливаются. Job переходит в состояние Cancelled.
Ваш пример не проще. Вы реализуете специальную конструкцию, где у основного метода run нужно ещё не забывать вызывать task.Tick(), тем самым вы фактически переизобретаете корутины, только корявого вида.
Ещё раз хочу обратить Ваше внимание, что контекст не имеет отношения к многозадачности, это лишь способ переноса информации.
Возвращаясь опять к вашему примеру, нет, вам не удастся со своей библиотекой сосредоточиться на бизнес-логике. Во, первых вам надо будет помнить про упомянутый выше Tick, что уже завязывает вашу логику на библиотеку. Во вторых, вы не сможете ничего заканселить, если внутри метода у вас будет например http вызов: вам придется городить внутри сложный механизм и все равно в итоге вы начнёте работать с context.Context.
В стандартном гошном подходе вы бы просто написали несколько функций с контекстом в качестве первого параметра и в основном методе запустили бы их в корутинах, в вашем же случае — попытка сделать что-то похожее не генераторы в php.
Да, и паниковать при ошибках плохая практика.
Вы попробуйте написать код для отмены сразу 7 сопроцедур а потом скажите насколько Ваше решение c использованием Context не сложнее моего.
Ещё раз хочу обратить Ваше внимание, что контекст не имеет отношения к многозадачности, это лишь способ переноса информации.
Именно поэтому его используют в: which carries deadlines, cancellation signals, and other request-scoped values across API boundaries and between processes.
Во-первых, я искренне восхищён тем уровнем сложности, который вы смогли привнести в простой на первый взгляд проект.
Во-вторых… посыпаю голову пеплом, но честно признаю — я нихрена не понял как бы это не было смешно :))).
Общее впечатление от кода — группа абстракций для абстрактного управления абстрактными абстракциями.
Поэтому озвучу свои мысли, а вам уж дальше думать стоит ли на них реагировать:
1. Assert'ы с выкидыванием panic() — не Go way (об этом в отдельном комментарии уже писал).
2. Слишком много интерфейсов и применяются они некорректно.
3. Свой… странный network manager (какова его цель)?
4. Собственная реализация кооперативной многозадачности через task.Tick() — реализация понятна, но непонятно зачем оно. Есть подозрение, что граблей там очень много закопано — достаточно одной таске начать делать что-то тяжеловесное и у вас весь eventloop начнёт на ней тормозить.
5. Огромное нагромождение каналов, явно больше, чем нужно.
По самому последнему пункту про контекст.
Классическое его применение — для остановки связанных горутин через CancelFunc(), а также для остановки синхронных функций (к примеру, net.DialContext).
p.s. Под виндой не собирается, забыли сделать import («net») в файле conncheck_win.go
https://github.com/AgentCoop/go-work/blob/main/task.go#L86 — не стоит пытаться управлять планеровщиком. Тем более, коль было чтение канала, он и так переключился. Даже дважды переключится в итоге.
ch := time.After(j.timeout)
for {
select {
case <-ch:
j.statemux.RLock()
state := j.state
j.statemux.RUnlock()
switch {
case state == RecurrentRunning, state == OneshotRunning:
j.Cancel(ErrJobExecTimeout)
}
return
}
}Не понял назначения таких конструкций. Это эквивалентно одиночному чтению из канала без select, case, for. Однако такой паттерн повторяется многократно.
Если оплата за количество строчек кода, то почему бы и нет ;))
p.s. Если что, это был сарказм.
Ну если написание прокси сервера — это простой по Вашему мнению проект, тогда Вам определенно нужно повторить успех Игоря Сысоева ;) Осталось только дописать отдачу статики и реализовать конфигуратор.
1. panic используются в Go для обработок ошибок, не понимаю что значит не Go way, если она родная функции библиотеки Go.
2. В конкретном проекте всего два интерфейса. Я уже говорил об этом в других комментариях к этой статье.
3. Ничего странного на мой взгляд. Простая библиотека для работы с сетевыми соединениями, к тому же еще сырая. Речь не о ней.
4. Нет никакой собственной реализации. В библиотеке используются стандартные механизмы и пакеты «sync» и «runtime».
5. Опять таки, «огромное» — понятие субъективное. В Вашем понимании оно выражается в количественном виде, но если я спрошу какое количество каналов на единицу кода считается уже огромным Вы не сможете мне дать ответ. Посмотрите на кабину современных пассажирских самолетов: огромное количество кнопок, экранов и переключателей. Инженеры тоже по Вашему неправы или так сделано потому что это вытекает из требований и сложности предметной области? В моем случае каналы используются для того для чего их создавали разработчики:
Do not communicate by sharing memory; instead, share memory by communicating.
ps
спасибо, исправлю.
- Нет. Паника для обработки ошибок в го не используется. Панику нужна для обработки критических ситуаций, собственно паник.
- Вы уверены, что понимаете, как работает runtime? И что с errgroup?
- Отвлеченные рассуждения. Ваш кад сложный. Он сложнее стандартных решений на порядок. Чем это оправдано?
Ну если написание прокси сервера — это простой по Вашему мнению проект, тогда Вам определенно нужно повторить успех Игоря Сысоева ;)
Не могу найти связи между вашей реализацией прокси и отсылкой к nginx.
Функционал прокси — простой функционал, nginx выстрелил благодаря набору своих на тот момент уникальных характеристик и возможности выдавать огромные скорости на минимальных ресурсах.
1. panic используются в Go для обработок ошибок, не понимаю что значит не Go way, если она родная функции библиотеки Go.
panic используется для обработки фатальной ошибки, которая не позволяет приложению продолжать выполнение.
…
5. Опять таки, «огромное» — понятие субъективное. В Вашем понимании оно выражается в количественном виде, но если я спрошу какое количество каналов на единицу кода считается уже огромным Вы не сможете мне дать ответ.
Да, субъективное.
Но, к примеру, конструкции:
stream.recvDataFrameChan < — frameи
<-stream.recvDataFrameSyncChan
stream.recvRawChan < — data
<-stream.recvRawSyncChan
очень режут глаз.
В чём смысл их применения? Зачем тут каналы?
Почему тут не синхронная функция (раз уж вы в любом случае завешиваете поток выполнения до получения данных из другого канала)?
p.s. Остальные пункты опущены осознанно, у каждого своё мнение, с вашим мнением не согласен, но готов его уважать :)
panic используются в Go для обработок ошибок, не понимаю что значит не Go way, если она родная функции библиотеки Go.В Go следует различать состояние ошибки, когда что-то пошло не так и это состояние можно обработать. Например, вернуть из метода или функции значение ошибки и всегда проверять все возвращаемые ошибки (err).
Помимо это может быть исключительная (невозможная) ситуация. И в случае такой ситуации следует использовать функцию panic.
Примеры:
состояние ошибки, когда что-то пошло не так — не получается открыть файл по той или иной причине;
исключительная ситуация — у вас было условие, которое всегда должно быть истинным, неожиданно стало ложным.
Ещё один важный момент. Вызов функции panic приводит к прекращению работы программы с выдачей трассировки для отладки. Поэтому рекомендуют использовать эту функцию на начальных этапах разработки.
habr.com/ru/post/537600/#comment_22554278
Не понимаю почему большинство программистов Go наделяют panic какими-то магическими свойствами. Это стандартная функция библиотеки Go и в документации к ней нигде не написано что она должна использоваться ТОЛЬКО лишь для завершения программы или ТОЛЬКО для обработки специального рода ошибок.
Для статьи стоило бы показать пример решения вашей задачи на стандартных механизмах голанга и потом с помощью вашей библиотеки. Пока выглядит как переусложнение там, где не нужно. Паттерны разумеется в го применимы, и command, и job, и любые другие, но как и всегда, если это обосновано чем-то
я никогда не был на собеседовании, устраиваясь на вакансию Go разработчика, но подозреваю, что там не задают вопросов про SOLID, Dependency Injection и различные прелести ООП
Это не си, без понимания SOLID, DI и ООП в го далеко не уехать (разве что на самописном велосипеде, который не покрыть тестами и не доработать без боли).
Выполнение задач зависит друг от друга, если одна задача прерывает свое выполнение из-за ошибки — все остальные задачи останавливаются тоже…
Для этого уже есть errgroup. Решает те же задачи, но в более идиоматичном для Go виде.
Простите, но
- Зачем пакету интерфейсы?
- Если точно еужны, то зачем большие интерфейсы?
- Если совсем точно нужны, то зачем публичные интерфейсы?
- Зачем пустые интерфейсы?
- Точно нужно столько горутин?
If a type exists only to implement an interface and will never have exported methods beyond that interface, there is no need to export the type itself. Exporting just the interface makes it clear the value has no interesting behavior beyond what is described in the interface. It also avoids the need to repeat the documentation on every instance of a common method.
2. Если Вы можете разбить эти «большие» интерфейсы на более мелкие, то сообщите пожалуйста как. Понятие «большой» интерфейс весьма субъективное. Я так понимаю оно у многих измеряется в попугаях.
3. Не совсем понял вопрос.
4. тоже самое
5. Сопроцедур нужно ровно столько сколько нужно. Их создание в Go относительно недорогая операция.
А еще в тех же правилах го то, что без необходимости мы интерфейсы не вводим. И далее я вас отсылаю к Interface pollution https://rakyll.org/interface-pollution/
https://www.ardanlabs.com/blog/2016/10/avoid-interface-pollution.html
Зачем эти интерфейсы пользователю? Если ему нужны интерфейсы, то он может их объявить на своей стороне. Благо у нас duck typing.
И горутины довольно дороги. Для сервисов под нагрузкой всегда стоит управлять и их количеством и временем жизни.
Лучшим решением было бы сделать горутины обработчики.
Я Вас отсылаю опять по ссылке в своем комментарии выше где черному по белому написано:
If a type exists only to implement an interface and will never have exported methods beyond that interface, there is no need to export the type itself. Exporting just the interface makes it clear the value has no interesting behavior beyond what is described in the interface. It also avoids the need to repeat the documentation on every instance of a common method.и мы бегаем по кругу.
Функциональность у вас типы задают. Интерфейсы реализуют строго по одному типу. В этих условиях интерфейсы не нужны.
Я тоже пришел из php — прислушайтесь к комментариям, тут по делу говорят. Пока у вас не гошный код совсем. Это php или java.
Но можете оставаться "как есть" со своим уникальным мнением и неконкурентом на гошном рынке труда. Тоже хорошо.
Я извиняюсь, но когда это горутины стали дорогими?
Со сменой контекста для исполнения горутин.
Большое количество горутин приводит к переизбытку горутин в каждом процессе планировщика. И каждой горутине требуется дать свой квант времени.
Тут опасность в безконтрольном создании горутин — ни разработчик пакета, ни пользователь уже не смогут сказать, сколько горутин конкурируют за ресурсы. И вот это уже дорого.
Работая в Лазаде, мы постепенно пришли к тому, что любой сервис стоит сначала писать простым и линейным, без конкурентрости. И только затем, если профилирование показывает плохое потребление ресурсов, добавлять конкурентную обработку. Это как с преждевременной оптимизацией.
Мы тоже сначала пихали везде горутины, думая, что так быстрее, а ресурсы лучше утилизируются. Однако позже вырезали лишнюю и неконтролируюмую конкурентность. И каждый раз получали выигрыш производительности.
Фиксированное число горутин обработчиков, читаюших канал задач, будут качественно лучше и быстрее, чем одна задача — одна горутина.
Вот тут отчасти затронута эта тема https://www.ardanlabs.com/blog/2018/12/scheduling-in-go-part3.html
Интерфейсы в го в большинстве случаев стоит делать только на стороне потребителя.
Т.е. вполне можно иметь просто тип Task, так как у вас нет разных типов задач, требующих обобщения. Если же в будущем они появятся — можно будет ввести интерфейс.
А пользователь вашей библиотеки вполне может у себя завести интерфейс с нужными ему методами и использовать его в меру необходимости, так как в отличии от php в го применим duck-typing
Тут asset через панику в рожу пользователю реализуется https://github.com/AgentCoop/go-work/blob/main/task.go#L96
Что тут ещё обсуждать?
Посмотрите на реализацию errgroup, сторонних ещё пакетов товарищей из гугла. Поучитесь.
Паники не должны использоваться никогда. Вообще.
Уж коль вы любите ссылки https://go-proverbs.github.io/
Паники не должны использоваться никогда. Вообще.
Не соглашусь.
Паниковать очень даже можно, если программа должна аварийно завершиться до момента запуска требующих graceful shutdown активностей.
К примеру, вызов panic() при ошибке чтения конфиг файла или невозможности подключиться к БД (при первоначальном старте приложения, когда реконнект не нужен) — правильное применение паники.
К примеру (отсюда: golangbot.com/panic-and-recover):
==
The idiomatic way of handling abnormal conditions in a Go program is using errors. Errors are sufficient for most of the abnormal conditions arising in the program.
But there are some situations where the program cannot continue execution after an abnormal condition. In this case, we use panic to prematurely terminate the program.
==
Панику можно и нужно применять тогда, когда «шеф, усё пропало», т.е. в случаях, когда приложение должно аварийно завершиться.
К примеру, будет нормой кинуть панику при аварийном завершении из-за ошибки парсинга конфигурационного файла. Либо по любой другой причине, которая не позволит приложению продолжать работать.
Вы же pinic()… recover() используете вместо try {… } catch (Exception E) {… } блока в Java.
Считается, что «это не Go way», так делать не принято, но сам по себе язык позволяет такое сделать :)
Хороший, развернутый комментарий.
Согласен полностью.
В общем я вижу тут очередной goto только в другой ипостасьи.
Панику можно и нужно применять тогда, когда «шеф, усё пропало»
test.Assert и есть тот случай, когда ошибка не позволяет другим задачам продолжать свое выполнение.
Религия тоже предписывает не кушать и не пить во время поста. Я же используя те возможности языка, которые дают сами же авторы особо не злоупотребляя чем либо.
Совсем уже ушли от темы разговора, но если вы устроились на должность священника, то кушать во время поста действительно нельзя. Даже если сами авторы уточняют, что некоторым категориям граждан кушать всё-таки можно и вы очень сильно хотите приписать себе характеристики тех самых граждан :)
test.Assert и есть тот случай, когда ошибка не позволяет другим задачам продолжать свое выполнение.
Самый последний раз, паника — она для прекращения работы исполняемого файла.
recover придуман для тех случаев, когда «ну очень-очень хочется и очень-очень надо», либо для в какой-то мере экзотических случаев, когда чужая библиотека считает, что надо крашить всё приложение, а приложение категорически не согласно с библиотекой и знает выход из ситуации.
В любом случае, спасибо за статью.
Полемику можно прекращать.
Работа по написанию статьи проделана большая, сама статья написана интересно, спасибо вам за это.
А то, что некоторые (включая меня) с вами категорически не согласны — так это нормально, всегда найдутся те, кто с чем-то не согласен :)))
Самый последний раз, паника — она для прекращения работы исполняемого файла.
Как-то ваши слова не совсем вяжутся с применением panic самими же авторами Go
blog.golang.org/defer-panic-and-recover
For a real-world example of panic and recover, see the json package from the Go standard library. It encodes an interface with a set of recursive functions. If an error occurs when traversing the value, panic is called to unwind the stack to the top-level function call, which recovers from the panic and returns an appropriate error value (see the 'error' and 'marshal' methods of the encodeState type in encode.go).
всегда найдутся те, кто с чем-то не согласен
Это да.
Давайте:
- Зачем паники?
- Зачем интерфейс Task? Публичный?
- Зачем в нём asset, который с паникой?
- Зачем паттерн вида
for { select { case <-s: j.prereqWg.Done() return } }
Если он эквивалентен
<-s
j.prereqWg.Done()
return- Зачем попытки управлять шедулерлм через runtime, если и так было чтение из канала?
- Зачем так сложно?
- Чем это лучше wg или errgroup? Если даже по количеству кода меньше будет?
- Проверки в тестах строятся на time.Sleep — https://github.com/AgentCoop/go-work/blob/main/job_test.go
- Зачем столько горутин? Чем это оправдано? Какую задачу решает?
- Это точно код на go?
Кроме шуток, прикрутите стандартные линтеры и прогоните через них свой код. Даже это сделает его качественно лучше.
Job — шаблон проектирования для новичков и опытных Go программистов