Optimize time of sess.gremlin and sess.close #170
Conversation
| def _launch_interactive_instance_impl(self): | ||
| try: | ||
| sess = get_session_by_id(self.session_id) | ||
| sess.gremlin(self) |
There was a problem hiding this comment.
Maybe we should add a option to control whether automatically create interactive instance whenever a graph is loaded
Reason: If always create, when user want to get the interactive instance, they call sess.gremlin(graph, params), and the method sess.gremlin() will simply find in the cache and return, the params is silently ignored, which may be confusing. Also since user can never reach the logic of creating instance, it should be in another private method.
There was a problem hiding this comment.
There may be another circumstance that user can close a previous instance, and create a new one. We allow that. And this procedure of course will takes about ~30s. To prevent user wondering why two gremlin call consumed very different time, I think you should add the silent thread creating technique to somewhere in the doc, and in the session/graph comments.
There was a problem hiding this comment.
Add auto_create_interactive_engine in config.py. Defaults to True.
There was a problem hiding this comment.
To prevent user wondering why two gremlin call consumed very different time, I think you should add the silent thread creating technique to somewhere in the doc, and in the session/graph comments.
Apart from the doc, when user run .gremlin(sess, params) with params, (we know the params are ignored), maybe we should print a warning to remind user that the params is not taken effect. And let them know the backend behavior.
i.e., logger.warning('params is ignored since xxx. see more in /path/to/doc/section/')
We should also tell them how to make params take effect without start the session from the beginning. I could come up with close previous interactive instance, and create a new one with params by call gremlin() manually. Please check the close-and-reopen procedure will behave as expected.
There was a problem hiding this comment.
Nice advice. I have updated doc of gremlin, which clarify how to create a new instance under the same graph with different engine_params.
I didn't add logger.warning in code, as gremlin may run in a thread and hidden from users.
There was a problem hiding this comment.
A new issue opened to trace InteractiveQuery create failed after close.
#172
lidongze0629
force-pushed
the
dz/profiling
branch
from
c8a5fe3 to
ae6701c
Compare
Codecov Report
@@ Coverage Diff @@
## main #170 +/- ##
===========================================
- Coverage 78.57% 66.05% -12.52%
===========================================
Files 51 50 -1
Lines 4606 4472 -134
===========================================
- Hits 3619 2954 -665
- Misses 987 1518 +531
Continue to review full report at Codecov.
|
|
|
||
| nest_asyncio.apply() | ||
| except: # noqa: E722 | ||
| pass |
There was a problem hiding this comment.
Why restore this change?
| # Otherwise, you should create a GIE instance manually by `sess.gremlin` if | ||
| # `initializing_interactive_engine` is False | ||
| initializing_interactive_engine = True | ||
|
|
There was a problem hiding this comment.
I'm wondering if the GSConfig docs could be rendered in sphinx properly.
There was a problem hiding this comment.
We didn't expose GSConfig in Doc. Instead, graphscope.set_option and graphscope.get_option exposed.
https://graphscope.io/docs/reference/generated/graphscope.set_option.html#graphscope.set_option
| def closed(self): | ||
| """Return if the current instance is closed.""" | ||
| return self._closed | ||
| return self._status == InteractiveQueryStatus.Closed |
There was a problem hiding this comment.
Since you have three status type, what will happen if the status is initializing and execute is called?
Will the call be blocked? or error out?
ref:
GraphScope/python/graphscope/interactive/query.py
Lines 139 to 141 in 10f36b1
There was a problem hiding this comment.
excute subgraph and traversal_source can only run under running status. Otherwise, raise an exception.
pass waiting for delete to InteravtiveQuery
lidongze0629
force-pushed
the
dz/profiling
branch
from
59ce132 to
7b4b487
Compare
What do these changes do?
sleepoperations during process of gremlin launchingRelated issue number
Fixes #157