Skip to content
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.

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

track each subquery field proc separately #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kindermax
Copy link
Collaborator

This fix aims to improve subquery fields tracking.

Problem

Let's consider case when several fields are resolved using same subgraph. In this case both fields will have the same duration (from superSlowField), but this is wrong.

Field('id', Integer, user_sg.c(get_user_id(S.this.id))),
Field('superSlowField', Integer, user_sg.c(get_super_slow_field(S.this.id))),

In production it looks like this:

image

Solution

We need to track each field resolver individually. In this pr wrap_proc will wrap field's proc function (proc function is basically a function with @define decorator on it) so instead of tracking result = result_proc() which is a _create_result_proc we will track each proc individually, see:

def _create_result_proc(
engine_query: Query, procs: List[Callable], options: List
) -> Callable[[], List[List]]:
def result_proc() -> List[List]:
sq_result = engine_query.result()
return [[proc(this, sq_result, *opt_args)
for proc, opt_args in zip(procs, options)]
for this in sq_result[THIS]]
return result_proc

Result

Simple example which prints observe args

Before:

--- OBSERVE Product id 0.6453857090018573
--- OBSERVE Product residueStatus 0.6453857090018573
--- OBSERVE Product residueColorStatus 0.6453857090018573
--- OBSERVE Product price 0.6453857090018573
--- OBSERVE Product currencyText 0.6453857090018573
--- OBSERVE Product _wholesale_prices 0.6453857090018573
--- OBSERVE Product wholesalePrices 3.5832999856211245e-05

After:

--- OBSERVE Product id 1.0446036259963876
--- OBSERVE Product residueStatus 1.0446859590010718
--- OBSERVE Product residueColorStatus 1.0447400010016281
--- OBSERVE Product price 1.0447855009988416
--- OBSERVE Product currencyText 1.0451795420012786
--- OBSERVE Product _wholesale_prices 1.1229197930006194
--- OBSERVE Product wholesalePrices 3.524999920045957e-05

def wrapper(field_names, *args):
def wrap_proc(start_time, field_name, proc):
def _proc_wrapper(*args):
result = proc(*args)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

passing start_time from above allows to track subgraph execution + proc execution time.

Copy link
Collaborator Author

@kindermax kindermax Feb 20, 2023

Choose a reason for hiding this comment

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

maybe it makes sense to place start_time = time.perf_counter() inside _proc_wrapper but then we are (probably) loosing subgraph(low-level graph) data exection time

m.kindritskiy added 2 commits March 25, 2023 14:46
the idea is that we track field with low level graph time + separate metric for just @define func so we can see how much time spent both low_level + @define and just @define
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant