HH-208976 refactor master-worker pipes #691
Conversation
frontik/app.py
Outdated
| self.worker_state = WorkerState(init_workers_count_down, count_down_lock) # type: ignore | ||
| self.service_discovery = get_sync_service_discovery(self.statsd_client) | ||
| self.upstreams: dict[str, Upstream] = {} | ||
| self.upstream_manager: UpstreamManager |
There was a problem hiding this comment.
как-то всё выглядит неуютно в в кучу
почему не может быть один объект отвечающий за всю балансировку - UpstreamManager
сейчас upstreams, upstream_manager, upstream_caches, service_discovery - раз ты рефакторишь - почему этому всему не жить в UpstreamManager?
| @@ -226,10 +229,12 @@ def next_request_id() -> str: | |||
| return FrontikApplication.request_id | |||
|
|
|||
| def get_current_status(self) -> dict[str, str]: | |||
| if self.init_workers_count_down.value > 0: | |||
| not_started_workers = self.worker_state.init_workers_count_down.value | |||
| master_done = self.worker_state.master_done.value | |||
There was a problem hiding this comment.
видимо раньше это под слоустартом было спрятано (который нужен ли в питоне?)
суть что пока мастер не забрал апстримы из консула и не отдал воркерам мы не должны начинать принимать запросы
| statsd_client: Union[StatsDClient, StatsDClientStub], | ||
| event_loop: Optional[BaseEventLoop] = None, | ||
| upstreams_lock: Optional[Lock], |
There was a problem hiding this comment.
хотел чтобы здесь вообще ничего из мультипроцессинга не было, но только так получилось
мы тут заряжаем консул коллбэки которые меняют upstreams
а в параллельном треде upstreams отправляются воркерам
и есть еще один тред который делает переотправку
соотв читать и изменять апстримы надо под локом
712u3
force-pushed
the
HH-208976
branch
2 times, most recently
from
9a9c3db
to
b9762bf
Compare
| while True: | ||
| self._resend_notification.get() | ||
| time.sleep(1.0) | ||
| def update_upstreams(self, upstreams: list[Upstream]) -> None: |
There was a problem hiding this comment.
тут без лока
это для воркера функция реагирования на апдейт апстримов с мастера
перетащил из баланс клиента, там это не нужно
frontik/app.py
Outdated
| self.statsd_client: Union[StatsDClient, StatsDClientStub] = create_statsd_client(options, self) | ||
|
|
||
| init_workers_count_down = multiprocessing.Value('i', options.workers) | ||
| master_done = multiprocessing.Value('i', 0) |
There was a problem hiding this comment.
master_done - не boolean-ом надо объявить?
frontik/process.py
Outdated
|
|
||
| def sigterm_handler(signum, frame): | ||
| if not state.server: | ||
| def sigterm_handler(signum, _frame): |
There was a problem hiding this comment.
sigterm_handler лучше назвать master_sigterm_handler, чтобы было ясно, что это не про воркер
frontik/server.py
Outdated
| app.upstream_manager.register_service() | ||
|
|
||
|
|
||
| def _worker_listener_handler(app: FrontikApplication, data: Any) -> None: |
There was a problem hiding this comment.
в data там реально Any прилетает или мб байты какие-то?
| ) -> None: | ||
| while True: | ||
| resend_notification.get() | ||
| time.sleep(1.0) |
There was a problem hiding this comment.
за что этот слип отвечает?
There was a problem hiding this comment.
хз) так предшественники сделали
с натяжкой вот такое объяснение могу дать
мастер пишет в пайп (чтоэто), воркер читает
у пайпа есть буффер некоторого размера
если воркер почемуто (затупил) и не читает, то буфер кончается
в мастере эксепшн случается про чето блокед и мы отмечаем вот эту resend движуху
если мы моментально ресенд сделаем то мы ничего не запишем тк воркер еще не разобрал буфер,
думаем видимо что через секунду он раздуплится
frontik/dependency_manager.py
Outdated
| 'headers': '', | ||
| 'handler': handler, | ||
| }) | ||
| request = Request({'type': 'http', 'query_string': '', 'headers': '', 'handler': handler}) |
There was a problem hiding this comment.
а зачем все эти переделки? это автоформатер теперь такой? как будто возможность упралять правилами переноса за счет запятой в конец была полезной
There was a problem hiding this comment.
реформаттер
можно попробовать поиграть настройками
There was a problem hiding this comment.
хотя хз че крутить
мне нравится тема что если на одну строку влезает то пусть на одной строке будет
There was a problem hiding this comment.
эт плохая тема имхо, бывает, что влезает, но но корректнее отдельной строкой
black вообще говоря специально позволял задавать самому переносы
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#trailing-commas
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma
мб как-то split-on-trailing-comma=true выставить или типа того?
|
SonarQube Quality Gate
|
https://jira.hh.ru/browse/HH-208976
рефактор пайпов