A Notebook submitter to run Jupyter Notebook inside cluster #13
Conversation
There was a problem hiding this comment.
If the client dies (e.g.: because SSH session dies), what prevents the YARN application from running forever? I think the AM should also be configured so that it automatically exits after some reason amount of time, if the TonYClient does not kill it before.
Can you provide a README with an example of how to submit the notebook with all the arguments required?
tony-proxy/src/main/java/com/linkedin/tonyproxy/ProxyServer.java
Outdated
Show resolved
Hide resolved
tony-proxy/src/main/java/com/linkedin/tonyproxy/ProxyServer.java
Outdated
Show resolved
Hide resolved
tony-core/src/main/java/com/linkedin/tony/TonyApplicationMaster.java
Outdated
Show resolved
Hide resolved
tony-cli/src/main/java/com/linkedin/tony/cli/NotebookSubmitter.java
Outdated
Show resolved
Hide resolved
tony-cli/src/main/java/com/linkedin/tony/cli/NotebookSubmitter.java
Outdated
Show resolved
Hide resolved
tony-proxy/src/main/java/com/linkedin/tonyproxy/ProxyServer.java
Outdated
Show resolved
Hide resolved
| @@ -48,6 +46,8 @@ | |||
|
|
|||
| public static final String WORKER_JOB_NAME = "worker"; | |||
| public static final String PS_JOB_NAME = "ps"; | |||
| public static final String NOTEBOOK_JOB_NAME = "Notebook"; | |||
| public static final String DRIVER_JOB_NAME = "Driver"; | |||
There was a problem hiding this comment.
Not sure I understand what this is used for?
There was a problem hiding this comment.
This is used for sending the address:port of notebook back to TonyClient
oliverhu
force-pushed
the
notebookSubmitter
branch
from
eaa5d28
to
84c9952
Compare
There was a problem hiding this comment.
Right now, client.cleanUp() calls amRpcClient.finishApplication() which sets shouldExit = true in the AM. However, this is only checked in the AM's stop() method, We should also check shouldExit in the monitor() loop to prevent the application from running forever even when the client dies.
Feel free to file an Issue for this and address in a separate PR if you like.
| private static final String ARCHIVE_PATH = "tf_archive.zip"; | ||
| private Configuration tonyConf; | ||
| private final long clientStartTime = System.currentTimeMillis(); | ||
| private Path appResourcesPath; | ||
| private int hbInterval; | ||
| private int maxHbMisses; | ||
|
|
||
| // For access from CLI. | ||
| public Set<TaskUrl> taskUrls = new HashSet<>(); |
There was a problem hiding this comment.
Can we add a getter for taskUrls that wraps the Set<TaskUrl> in a Guava ImmutableSet to prevent manipulation by code outside this class?
There was a problem hiding this comment.
this doesn't look performant tho? You'd have to use ImmutableSet.copyOf(xxx) which copies everything ?
There was a problem hiding this comment.
True, it does copy everything, but I doubt this will ever be the performance bottleneck. Correctness/safety first, then performance.
oliverhu
force-pushed
the
notebookSubmitter
branch
from
f577cfd
to
eb5ee9e
Compare
|
validated functionality again after all the updates and it works, merging it! |
No description provided.