[FLINK-13793][docs] build each language in a separate subprocess #9489

NicoK · 2019-08-20T08:34:29Z

What is the purpose of the change

When not serving docs and just building them, e.g. via ./build_docs.sh, we can spawn sub-processes for each language build instead of running them single-threaded (jekyll currently does not support parallel builds).

before: ~37s
after: ~20s

Brief change log

Building on top of #9487, this PR adds:

change build_docs.sh to support parallel builds and merge resulting files together
(admittedly this seems a bit hacky)
enhance build_docs.sh: show usage if wrong option is given
exclude build scripts from generated contents

Verifying this change

I verified the changes in the generated HTML pages (nothing changed except for the build scripts which I removed from the generated pages via a hotfix).

flinkbot · 2019-08-20T08:37:24Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit a687283 (Wed Dec 04 15:08:41 UTC 2019)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2019-08-20T08:41:52Z

CI report:

b12cae9 Travis: CANCELED
b2ef697 Travis: SUCCESS
a687283 Travis: SUCCESS
b1da957 Travis: SUCCESS Azure: SUCCESS
813e35e Travis: FAILURE Azure: SUCCESS
78371b9 Travis: FAILURE Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build
@flinkbot run azure re-run the last Azure build

NicoK · 2019-11-14T15:54:00Z

I just rebased this PR now that we haveJekyll 4 changes in from #9487.

zentol · 2019-12-12T21:37:50Z

docs/build_docs.sh

@@ -72,6 +72,8 @@ while getopts "piez" opt; do
 		z)
 		JEKYLL_CONFIG="--config _config.yml,_config_dev_zh.yml"
 		;;
+    *) echo "usage: $0 [-e|-z] [-i|-p]" >&2


indentation is different

zentol · 2019-12-12T21:38:51Z

docs/_config_dev_en.yml

@@ -17,3 +17,6 @@

 exclude:
  - "*.zh.md"
+  - "content"


are these required for correctness?

if not excluded, they may end up in the generated folder (as yet another subdirectory) again, e.g. when the zh version builds, if would not see the content_en as a target dir (and exclude it)

zentol · 2019-12-12T21:42:47Z

docs/build_docs.sh

@@ -78,4 +80,28 @@ while getopts "piez" opt; do
 done

 # use 'bundle exec' to insert the local Ruby dependencies
-bundle exec jekyll ${JEKYLL_CMD} ${JEKYLL_CONFIG} --source "${DOCS_SRC}" --destination "${DOCS_DST}"
+
+if [ "${JEKYLL_CMD}" = "build" ] && [ -z "${JEKYLL_CONFIG}" ]; then


if we modified the e/z cases above to instead set DOC_LANGUAGES to en/zh we wouldn't need this condition nor else branch

maybe a branch for the DOC_LANGUAGES (to get all languages if none of the e/z options was used) but simpler anyway; I'll give it a try

oh, the branch is still always needed since there is no parallel serve...so not sure whether this change makes sense

zentol · 2020-02-06T14:40:24Z

@NicoK ping

zentol

What would happen if we started jekyll with --serve for the en documentation, and fire up a background process that builds and copies the chinese docs into DOCS_DST?

zentol · 2020-02-07T08:42:38Z

docs/build_docs.sh

+
+  # run processes and store pids
+  echo "Spawning parallel builds for ${DOC_LANGUAGES}..."
+  pids=


my god is this actually valid in bash? It looks soo much like an incomplete statement.

yes it is ;)
but I can also replace it with pids=""

NicoK · 2020-02-07T10:24:43Z

Serving en and building zh in the background would be a nice trick but the result depends on the timing of when the zh build finishes. If it is first, then the en build will remove the contents directory and replace it with its own (thus removing the zh files) :(

Verified by running this:

set -x
(set -x; bundle exec jekyll build --baseurl= --config _config.yml,_config_dev_zh.yml --source "`pwd`" --destination "`pwd`/content_zh" && mkdir -p "`pwd`/content" && cp -aln "`pwd`/content_zh/." "`pwd`/content" && rm -rf "`pwd`/content_zh") &
bundle exec jekyll serve --baseurl= --watch --incremental --config _config.yml,_config_dev_en.yml --source "`pwd`" --destination "`pwd`/content"

NicoK · 2020-02-07T10:29:04Z

Ideally, of course, these tricks would not be necessary and jekyll would just use more system resources, i.e. do a parallel build on its own. That would give an even higher speed-up but is, unfortunately, not available, afaik

When not serving docs and just building them, e.g. via `./build_docs.sh`, we spawn sub-processes for each language build instead of running them single-threaded (jekyll currently does not support parallel builds).

NicoK · 2020-02-07T15:12:40Z

FYI: I rebased onto latest master again and verified that the result from building the old way vs. this PR is the same

NicoK mentioned this pull request Aug 20, 2019

[FLINK-13791][docs] Speed up sidenav by using group_by #9487

Closed

NicoK force-pushed the f13793 branch from b12cae9 to b2ef697 Compare August 20, 2019 08:37

rmetzger added the review=description? label Aug 20, 2019

rmetzger added the component=Documentation label Aug 20, 2019

NicoK force-pushed the f13793 branch from b2ef697 to a687283 Compare November 14, 2019 13:54

NicoK requested a review from zentol November 28, 2019 10:36

zentol self-assigned this Dec 12, 2019

zentol reviewed Dec 12, 2019

View reviewed changes

zentol approved these changes Feb 7, 2020

View reviewed changes

NicoK force-pushed the f13793 branch from b1da957 to 813e35e Compare February 7, 2020 10:33

NicoK added 3 commits February 7, 2020 11:35

[hotfix][docs] show usage if wrong option is given

52d408d

[FLINK-13793][docs] build each language in a separate subprocess

381848e

When not serving docs and just building them, e.g. via `./build_docs.sh`, we spawn sub-processes for each language build instead of running them single-threaded (jekyll currently does not support parallel builds).

[hotfix][docs] do not include build scripts in content generation

78371b9

NicoK force-pushed the f13793 branch from 813e35e to 78371b9 Compare February 7, 2020 15:10

NicoK merged commit fcb443a into apache:master Feb 13, 2020

NicoK deleted the f13793 branch February 13, 2020 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-13793][docs] build each language in a separate subprocess #9489

[FLINK-13793][docs] build each language in a separate subprocess #9489

NicoK commented Aug 20, 2019

flinkbot commented Aug 20, 2019 •

edited

flinkbot commented Aug 20, 2019 •

edited

NicoK commented Nov 14, 2019

zentol Dec 12, 2019

zentol Dec 12, 2019

NicoK Feb 6, 2020

zentol Dec 12, 2019

NicoK Feb 6, 2020

NicoK Feb 6, 2020

zentol commented Feb 6, 2020

zentol left a comment

zentol Feb 7, 2020

NicoK Feb 7, 2020 •

edited

NicoK commented Feb 7, 2020 •

edited

NicoK commented Feb 7, 2020

NicoK commented Feb 7, 2020

[FLINK-13793][docs] build each language in a separate subprocess #9489

[FLINK-13793][docs] build each language in a separate subprocess #9489

Conversation

NicoK commented Aug 20, 2019

What is the purpose of the change

Brief change log

Verifying this change

flinkbot commented Aug 20, 2019 • edited

Automated Checks

Review Progress

flinkbot commented Aug 20, 2019 • edited

CI report:

NicoK commented Nov 14, 2019

zentol Dec 12, 2019

Choose a reason for hiding this comment

zentol Dec 12, 2019

Choose a reason for hiding this comment

NicoK Feb 6, 2020

Choose a reason for hiding this comment

zentol Dec 12, 2019

Choose a reason for hiding this comment

NicoK Feb 6, 2020

Choose a reason for hiding this comment

NicoK Feb 6, 2020

Choose a reason for hiding this comment

zentol commented Feb 6, 2020

zentol left a comment

Choose a reason for hiding this comment

zentol Feb 7, 2020

Choose a reason for hiding this comment

NicoK Feb 7, 2020 • edited

Choose a reason for hiding this comment

NicoK commented Feb 7, 2020 • edited

NicoK commented Feb 7, 2020

NicoK commented Feb 7, 2020

flinkbot commented Aug 20, 2019 •

edited

flinkbot commented Aug 20, 2019 •

edited

NicoK Feb 7, 2020 •

edited

NicoK commented Feb 7, 2020 •

edited