Skip to content

Update exposer's Redis retry exception handling #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aokhovat wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update exposer's Redis retry exception handling #35

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions polygen/qos_metrics_exposer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ def retrieve_data_from_redis(
except Exception as ex:
logger.warning(
f"failed to decode a redis key '{encoded_key!r}': {ex}"
) # type:ignore
) # type: ignore
continue

for decoded_key in decoded_redis_keys: # type:ignore
for decoded_key in decoded_redis_keys: # type: ignore
try:
metric = MetricService.create_user_level_metric(decoded_key, current_epoch)
if metric.metric_type == UsageType.VERB:
Expand Down Expand Up @@ -143,13 +143,18 @@ def retrieve_data_from_redis(
# main process that handles gathering data from redis-qos
#
def get_redis_server_connection(redis_server: str) -> redis.Redis:
host, port = redis_server.split(":")
while True:
try:
host, port = redis_server.split(":")
rs_conn = redis.Redis(host=host, port=int(port), db=0)
if not rs_conn.ping():
raise Exception("connection established but redis is not pinging")
return rs_conn
except redis.exceptions.ConnectionError as ex:

@jacquesh jacquesh Mar 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What non-ConnectionError exceptions are we expecting to catch here though? If its just the ping check above, could we not just remove that exception and replace it with a log, sleep & continue?

logger.warning(
f"failed to connect {redis_server}: {ex}; retrying in 1 second"
)
time.sleep(1)
except Exception as ex:
logger.exception(f"failed to connect {redis_server}: {ex}")
time.sleep(1)
Expand All @@ -158,6 +163,13 @@ def get_redis_server_connection(redis_server: str) -> redis.Redis:
MIN_SLEEP_TIME_SEC_PER_ROUND = 0.1 # 100 msec


def is_redis_connection_alive(rs_conn: redis.Redis) -> bool:
try:
return bool(rs_conn.ping())
except redis.exceptions.RedisError:
return False


def redis_consumer_process(
metrics_queue: "MetricsQueue", redis_server: str
) -> NoReturn:
Expand All @@ -167,7 +179,7 @@ def redis_consumer_process(

while True:
try:
if rs_conn is None or not rs_conn.ping():
if rs_conn is None or not is_redis_connection_alive(rs_conn):
rs_conn = get_redis_server_connection(redis_server)

start_time = time.time()
Expand Down