-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
[FLINK-5494] [docs] Add more details to the Mesos documentation #3236
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @tillrohrmann!
Followed them as a user, and it worked well. I have some suggestions on parts where I got a bit confused.
+1 to merge once they're addressed :-)
"cpus": 1.0, | ||
"mem": 2048, | ||
"mem": 1024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keys in this configuration example is missing indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added indentation.
} | ||
|
||
When running Flink with Marathon, the whole Flink cluster including the job manager will be run as Mesos tasks in the Mesos cluster. | ||
|
||
### Configuration parameters | ||
|
||
#### Mesos configuration entries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add the Mesos config to setup/config.md
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes they should also go there.
|
||
#### Mesos Library | ||
|
||
In order to run Java applications with Mesos you have to export `MESOS_NATIVE_JAVA_LIBRARY=MESOS_HOME/lib/libmesos.so`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Mac OSX, the native library name seems to be libmesos.dylib
. Might be good to inform in the docs what OS the instructions is assuming, or just add a small comment about Mac OSX here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Will add this.
presence of master node failures. Additionally, you probably want to install a | ||
distributed file system to share data across nodes and make use of Flink's | ||
checkpointing mechanism. | ||
Additionally, you probably want to install a distributed file system to share data across nodes and make use of Flink's checkpointing mechanism. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I bumped a bit on this sentence. The relationship between a distributed file system and Flink's checkpointing is a bit unclear here. Perhaps change to something like "use filesystem backend for Flink's checkpointing", and also link to the related documents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have removed the sentence.
|
||
### Installing Marathon | ||
|
||
Optionally, you may also [install Marathon](https://mesosphere.github.io/marathon/docs/) which will be necessary to run Flink in high availability (HA) mode. | ||
|
||
### Optional dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This title is a bit not synced with the other title changes: "Installing Marathon" was singled out if "Optional dependencies", even though Marathon is still optional.
Perhaps "Installing Maration (Optional)" and change this also to "Installing a Distributed Filesystem (Optional)".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove the DFS installation since it's not really related to Mesos.
following forms: host:port, zk://host1:port1,host2:port2,.../path, | ||
zk://username:password@host1:port1,host2:port2,.../path, | ||
file:///path/to/file (where file contains one of the above) | ||
`mesos.master`: The Mesos master URL. The value should be in one of the following forms: host:port, zk://host1:port1,host2:port2,.../path, zk://username:password@host1:port1,host2:port2,.../path, file:///path/to/file (where file contains one of the above) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this part a bit hard to read on the compiled docs. Perhaps use a bulleted list to show the different formats?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is a good idea. Will change it accordingly.
Thanks for your review @tzulitai. I've addressed your comments. Merging this PR now. |
Adds more details to the Mesos documentation so that setting up a Mesos cluster and running a Flink cluster in it becomes easier.