Add Session workflows documentation #808

andreyvelich · 2022-04-26T21:55:25Z

I added create and delete Session workflow to architecture diagrams.
Please take a look at the doc changes.

codecov-commenter · 2022-04-26T22:03:18Z

Codecov Report

Merging #808 (1d8d771) into main (241f0f2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #808   +/-   ##
=======================================
  Coverage   69.96%   69.96%           
=======================================
  Files          62       62           
  Lines        7368     7368           
  Branches     1223     1223           
=======================================
  Hits         5155     5155           
  Misses       1841     1841           
  Partials      372      372

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 241f0f2...1d8d771. Read the comment docs.

kevin-bates

Another useful diagram - thank you @andreyvelich! Here are some initial comments...

kevin-bates · 2022-04-26T22:10:54Z

docs/source/developers/architecture.rst

+Create Session Workflow
+-----------------------
+
+The create Session workflow can be seen in figure below:


Suggested change

The create Session workflow can be seen in figure below:

The create Session workflow can be seen in the figure below:

kevin-bates · 2022-04-26T22:11:37Z

docs/source/developers/architecture.rst

+   :width: 90%
+   :align: center
+
+When a user starts a new Kernel, the following steps process:


Suggested change

When a user starts a new Kernel, the following steps process:

When a user starts a new kernel, the following steps occur:

I think "Kernel" should be "kernel" except when it's a proper noun (e.g., Mapping Kernel Manager, Kernel Spec Manager, etc.). I believe this was also true in the general architecture section as well. Perhaps a sweep can be made for such instances.

Sounds good, I will change it to "kernel".

kevin-bates · 2022-04-26T22:16:28Z

docs/source/developers/architecture.rst

+   - When interrupt mode is ``Signal``, **Kernel Provisioner** kills the Kernel
+     with ``SIGINT`` operating system signal.


Suggested change

- When interrupt mode is ``Signal``, **Kernel Provisioner** kills the Kernel

with ``SIGINT`` operating system signal.

- When the interrupt mode is ``Signal``, the **Kernel Provisioner** interrupts the kernel

with the ``SIGINT`` operating system signal (although other provisioner implementations may use a different approach).

The diagram for the signal-based interrupt should probably replace Kill with Interrupt as well.

kevin-bates · 2022-04-26T22:37:11Z

docs/source/developers/architecture.rst

+#. When Kernel is shutdown, **Session Manager** deletes the Session data from
+   the SQLite3 DataBase and responses 204 Status Code to the Notebook client.


Should the kernel's shutdown be detailed similar to its interrupt? I know that can be a lot and we might be trying to limit text. Perhaps we could do something like...

When the kernel is shutdown, ...

and point to the code (or documentation produced from the code)?

@kevin-bates I think that makes sense. I added steps after kernel interrupt.
Please let me know, what do you think.

kevin-bates

Hi @andreyvelich - thanks for the updates! Just had a couple of minor comments regarding shutdown.

kevin-bates · 2022-04-27T14:48:01Z

docs/source/developers/architecture.rst

+#. **Mapping Kernel Manager** starts the kernel shutdown process by using
+   **Multi Kernel Manager** and **Kernel Manager**.
+
+#. **Kernel Manager** receives interrupt mode from **Kernel Spec Manager**.


Suggested change

#. **Kernel Manager** receives interrupt mode from **Kernel Spec Manager**.

#. **Kernel Manager** determines the mode of interrupt from the **Kernel Spec Manager**.

kevin-bates · 2022-04-27T14:48:15Z

docs/source/developers/architecture.rst

+   message on the control channel.
+
+#. **Kernel Manager** waits for the kernel shutdown. After the timeout,
+   **Kernel Manager** interrupts the kernel with ``SIGTERM`` operating system signal.


Suggested change

**Kernel Manager** interrupts the kernel with ``SIGTERM`` operating system signal.

**Kernel Manager** terminates the kernel sending a ``SIGTERM`` operating system signal (or provisioner equivalent). If it finds the kernel process has not terminated, the **Kernel Manager** will follow up with a ``SIGKILL`` operating system signal (or provisioner equivalent) to ensure the kernel's termination.

kevin-bates · 2022-04-27T14:49:41Z

docs/source/developers/architecture.rst

+   Control, and Heartbeat ports.
+
+#. When shutdown is finished, **Session Manager** deletes the Session data from
+   the SQLite3 DataBase and responses 204 Status Code to the Notebook client.


Suggested change

the SQLite3 DataBase and responses 204 Status Code to the Notebook client.

the SQLite3 database and responses 204 status code to the Notebook client.

kevin-bates · 2022-04-27T14:53:40Z

docs/source/developers/architecture.rst

+#. After interrupting kernel, Session sends the `"shutdown_request" <https://jupyter-client.readthedocs.io/en/latest/messaging.html#kernel-shutdown>`_
+   message on the control channel.
+
+#. **Kernel Manager** waits for the kernel shutdown. After the timeout,


Suggested change

#. **Kernel Manager** waits for the kernel shutdown. After the timeout,

#. **Kernel Manager** waits for the kernel shutdown. After the timeout, and if it detects the kernel process is still running, the

andreyvelich · 2022-04-27T15:42:04Z

Thank you for the review @kevin-bates!
I made these changes.

docs/source/developers/architecture.rst

kevin-bates

Looks good @andreyvelich - thank you!

Co-authored-by: Steven Silvester <[email protected]>

andreyvelich · 2022-04-27T16:57:10Z

Thank you for your feedback @blink1073 @kevin-bates !

blink1073

Thanks! Failure is unrelated

Add Session workflows documentation

f4d8b0a

kevin-bates reviewed Apr 26, 2022

View reviewed changes

Changes after review

052cf6f

kevin-bates reviewed Apr 27, 2022

View reviewed changes

Changes for Kernel Shutdown

b11e28c

blink1073 added the documentation label Apr 27, 2022

blink1073 reviewed Apr 27, 2022

View reviewed changes

docs/source/developers/architecture.rst Outdated Show resolved Hide resolved

docs/source/developers/architecture.rst Outdated Show resolved Hide resolved

docs/source/developers/architecture.rst Outdated Show resolved Hide resolved

kevin-bates previously approved these changes Apr 27, 2022

View reviewed changes

andreyvelich dismissed kevin-bates’s stale review via 49d5bde April 27, 2022 16:56

andreyvelich and others added 3 commits April 27, 2022 17:56

Update docs/source/developers/architecture.rst

49d5bde

Co-authored-by: Steven Silvester <[email protected]>

Update docs/source/developers/architecture.rst

a8cf76e

Co-authored-by: Steven Silvester <[email protected]>

Update docs/source/developers/architecture.rst

1d8d771

Co-authored-by: Steven Silvester <[email protected]>

blink1073 approved these changes Apr 27, 2022

View reviewed changes

blink1073 merged commit 3a91f36 into jupyter-server:main Apr 27, 2022

andreyvelich deleted the add-workflow-diagrams branch April 27, 2022 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Session workflows documentation #808

Add Session workflows documentation #808

andreyvelich commented Apr 26, 2022

codecov-commenter commented Apr 26, 2022 •

edited

Loading

kevin-bates left a comment

kevin-bates Apr 26, 2022

kevin-bates Apr 26, 2022

kevin-bates Apr 26, 2022

andreyvelich Apr 27, 2022

kevin-bates Apr 26, 2022

kevin-bates Apr 26, 2022

kevin-bates Apr 26, 2022

andreyvelich Apr 27, 2022

kevin-bates left a comment

kevin-bates Apr 27, 2022

kevin-bates Apr 27, 2022

kevin-bates Apr 27, 2022

kevin-bates Apr 27, 2022

andreyvelich commented Apr 27, 2022

kevin-bates left a comment

andreyvelich commented Apr 27, 2022

blink1073 left a comment

	The create Session workflow can be seen in figure below:
	The create Session workflow can be seen in the figure below:

	When a user starts a new Kernel, the following steps process:
	When a user starts a new kernel, the following steps occur:

		- When interrupt mode is ``Signal``, Kernel Provisioner kills the Kernel
		with ``SIGINT`` operating system signal.

		#. When Kernel is shutdown, Session Manager deletes the Session data from
		the SQLite3 DataBase and responses 204 Status Code to the Notebook client.

	#. Kernel Manager receives interrupt mode from Kernel Spec Manager.
	#. Kernel Manager determines the mode of interrupt from the Kernel Spec Manager.

	Kernel Manager interrupts the kernel with ``SIGTERM`` operating system signal.
	Kernel Manager terminates the kernel sending a ``SIGTERM`` operating system signal (or provisioner equivalent). If it finds the kernel process has not terminated, the Kernel Manager will follow up with a ``SIGKILL`` operating system signal (or provisioner equivalent) to ensure the kernel's termination.

	#. Kernel Manager waits for the kernel shutdown. After the timeout,
	#. Kernel Manager waits for the kernel shutdown. After the timeout, and if it detects the kernel process is still running, the

Add Session workflows documentation #808

Add Session workflows documentation #808

Conversation

andreyvelich commented Apr 26, 2022

codecov-commenter commented Apr 26, 2022 • edited Loading

Codecov Report

kevin-bates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-bates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreyvelich commented Apr 27, 2022

kevin-bates left a comment

Choose a reason for hiding this comment

andreyvelich commented Apr 27, 2022

blink1073 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 26, 2022 •

edited

Loading