You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/03/17 00:47:42 UTC
[GitHub] [beam] kileys opened a new pull request #17110: [WIP] Open modules for jamm
kileys opened a new pull request #17110:
URL: https://github.com/apache/beam/pull/17110
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [WIP] Open modules for jamm
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1069790626
Run Load Tests Java 17 CoGBK Dataflow V2 Batch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1071219474
Checked that there's no more warnings from the cache. I'll look to add an integration test for it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075606462
R: @damccorm @jrmccluskey
Can either of you review the Go code?
I'll review the Java changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1071171417
Run Load Tests Java 17 CoGBK Dataflow V2 Batch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075905862
Kiley, please follow up with Java 11 change, nits, and tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832596779
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowPipelineOptions.java
##########
@@ -217,4 +217,22 @@ public String create(PipelineOptions options) {
boolean isHotKeyLoggingEnabled();
void setHotKeyLoggingEnabled(boolean value);
+
+ /**
+ * Open modules needed for reflection that access JDK internals with Java 17+
Review comment:
```suggestion
* Open modules needed for reflection that access JDK internals with Java 9+
```
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/options/SdkHarnessOptions.java
##########
@@ -294,4 +295,18 @@ public static SdkHarnessLogLevelOverrides from(Map<String, String> values) {
return overrides;
}
}
+
+ /**
+ * Open modules needed for reflection that access JDK internals with Java 17+
Review comment:
```suggestion
* Open modules needed for reflection that access JDK internals with Java 9+
```
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/options/SdkHarnessOptions.java
##########
@@ -294,4 +295,18 @@ public static SdkHarnessLogLevelOverrides from(Map<String, String> values) {
return overrides;
}
}
+
+ /**
+ * Open modules needed for reflection that access JDK internals with Java 17+
+ *
+ * <p>With JDK 16+, <a href="#{https://openjdk.java.net/jeps/403}">JDK internals are strongly
+ * encapsulated</a> and can result in an InaccessibleObjectException being thrown if a tool or
+ * library uses reflection that access JDK internals. If you see these errors in your worker logs,
+ * you can pass in modules to open using the format module/package=target-module(,target-module)*
+ * to allow access to the library. E.g. java.base/java.lang=jamm
+ */
Review comment:
```suggestion
*
* <p>You may see warnings that jamm, a library used to more accurately size objects, is unable to
* make a private field accessible. To resolve the warning, open the specified module/package to
* jamm.
*/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832611975
##########
File path: sdks/java/container/boot.go
##########
@@ -193,7 +195,35 @@ func main() {
} else {
args = append(args, jammAgentArgs)
}
+ // Apply meta options
+ const metaDir = "/opt/apache/beam/options"
+ metaOptions, err := LoadMetaOptions(metaDir)
+ javaOptions := BuildOptions(metaOptions)
+ // (1) Add custom jvm arguments: "-server -Xmx1324 -XXfoo .."
+ args = append(args, javaOptions.JavaArguments...)
Review comment:
Is there any risk of duplicate args here? What would happen in that case/should we do some validation/warn or overwrite?
##########
File path: sdks/java/container/boot.go
##########
@@ -211,3 +241,120 @@ func heapSizeLimit(info *fnpb.ProvisionInfo) uint64 {
}
return 1 << 30
}
+
+// Options represents java VM invocation options in a simple,
+// semi-structured way.
+type Options struct {
+ JavaArguments []string `json:"java_arguments,omitempty"`
+ Properties map[string]string `json:"properties,omitempty"`
+ Classpath []string `json:"classpath,omitempty"`
+}
+
+// MetaOption represents a jvm environment transformation or setup
+// that the launcher employs. The aim is to keep the service-side and
+// user-side required configuration simple and minimal, yet allow
+// numerous execution tweaks. Most tweaks are enabled by default and
+// require no input. Some setups, such as Cloud Debugging, are opt-in.
+//
+// Meta-options are usually included with the image and use supporting
+// files, usually jars. A few are intrinsic because they are require
+// additional input or complex computations, such as Cloud Debugging
+// and Cloud Profiling. Meta-options can be enabled or disabled by
+// name. For the most part, the meta-option names are not guaranteed
+// to be backwards compatible or stable. They are rather knobs that
+// can be tuned if some well-intended transformation cause trouble for
+// a customer. For tweaks, the expectation is that the default is
+// almost always correct.
+//
+// Meta-options are simple additive manipulations applied in priority
+// order (applied low to high) to allow jvm customization by adding
+// files, notably enabling customization by later docker layers. The
+// override semantics is prepend for lists and simple overwrite
+// otherwise. A common use case is adding a jar to the beginning of
+// the classpath, such as the shuffle or windmill jni jar, or adding
+// an agent.
+type MetaOption struct {
+ Name string `json:"name,omitempty"`
+ Description string `json:"description,omitempty"`
+ Enabled bool `json:"enabled,omitempty"`
+ Priority int `json:"priority,omitempty"`
+ Options Options `json:"options"`
+}
+
+// byPriority sorts MetaOptions by priority, highest first.
+type byPriority []*MetaOption
+
+func (f byPriority) Len() int { return len(f) }
+func (f byPriority) Swap(i, j int) { f[i], f[j] = f[j], f[i] }
+func (f byPriority) Less(i, j int) bool { return f[i].Priority > f[j].Priority }
+
+// LoadMetaOptions scans the directory tree for meta-option metadata
+// files and loads them. Any regular file named "option-XX.json" is
+// strictly assumed to be a meta-option file. This strictness allows
+// us to fail hard if such a file cannot be parsed.
+//
+// Loading meta-options from disk allows extra files and their
+// configuration be kept together and defined externally.
+func LoadMetaOptions(dir string) ([]*MetaOption, error) {
+ var meta []*MetaOption
+
+ worker := func(path string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+ if !info.Mode().IsRegular() {
+ return nil
+ }
+ if !strings.HasPrefix(info.Name(), "option-") {
+ return nil
+ }
+ if !strings.HasSuffix(info.Name(), ".json") {
+ return nil
+ }
+
+ content, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
+ }
+
+ var option MetaOption
+ if err := json.Unmarshal(content, &option); err != nil {
+ return fmt.Errorf("failed to parse %s: %v", path, err)
+ }
+
+ log.Printf("Loaded meta-option '%s'", option.Name)
+
+ meta = append(meta, &option)
+ return nil
+ }
+
+ if err := filepath.Walk(dir, worker); err != nil {
+ return nil, err
+ }
+ return meta, nil
+}
+
+func BuildOptions(metaOptions []*MetaOption) *Options {
Review comment:
Could you please add tests for this and LoadMetaOptions? Both are pretty well contained/good candidates for units.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1071171417
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075681440
Run Dataflow Runner V2 Java 17 Nexmark Tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys removed a comment on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys removed a comment on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075668388
Dataflow Runner V2 Java 17 Nexmark Tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075600663
Run Load Tests Java 17 CoGBK Dataflow V2 Batch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075648144
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832743879
##########
File path: sdks/java/container/common.gradle
##########
@@ -108,3 +115,4 @@ if (project.rootProject.hasProperty(["docker-pull-licenses"]) ||
dockerPrepare.dependsOn copySdkHarnessLauncher
dockerPrepare.dependsOn copyDockerfileDependencies
dockerPrepare.dependsOn ":sdks:java:container:downloadCloudProfilerAgent"
+dockerPrepare.dependsOn copyJdkOptions
Review comment:
nit New line at eof
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075668388
Dataflow Runner V2 Java 17 Nexmark Tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r833175717
##########
File path: sdks/java/container/boot.go
##########
@@ -211,3 +241,120 @@ func heapSizeLimit(info *fnpb.ProvisionInfo) uint64 {
}
return 1 << 30
}
+
+// Options represents java VM invocation options in a simple,
+// semi-structured way.
+type Options struct {
+ JavaArguments []string `json:"java_arguments,omitempty"`
+ Properties map[string]string `json:"properties,omitempty"`
+ Classpath []string `json:"classpath,omitempty"`
+}
+
+// MetaOption represents a jvm environment transformation or setup
+// that the launcher employs. The aim is to keep the service-side and
+// user-side required configuration simple and minimal, yet allow
+// numerous execution tweaks. Most tweaks are enabled by default and
+// require no input. Some setups, such as Cloud Debugging, are opt-in.
+//
+// Meta-options are usually included with the image and use supporting
+// files, usually jars. A few are intrinsic because they are require
+// additional input or complex computations, such as Cloud Debugging
+// and Cloud Profiling. Meta-options can be enabled or disabled by
+// name. For the most part, the meta-option names are not guaranteed
+// to be backwards compatible or stable. They are rather knobs that
+// can be tuned if some well-intended transformation cause trouble for
+// a customer. For tweaks, the expectation is that the default is
+// almost always correct.
+//
+// Meta-options are simple additive manipulations applied in priority
+// order (applied low to high) to allow jvm customization by adding
+// files, notably enabling customization by later docker layers. The
+// override semantics is prepend for lists and simple overwrite
+// otherwise. A common use case is adding a jar to the beginning of
+// the classpath, such as the shuffle or windmill jni jar, or adding
+// an agent.
+type MetaOption struct {
+ Name string `json:"name,omitempty"`
+ Description string `json:"description,omitempty"`
+ Enabled bool `json:"enabled,omitempty"`
+ Priority int `json:"priority,omitempty"`
+ Options Options `json:"options"`
+}
+
+// byPriority sorts MetaOptions by priority, highest first.
+type byPriority []*MetaOption
+
+func (f byPriority) Len() int { return len(f) }
+func (f byPriority) Swap(i, j int) { f[i], f[j] = f[j], f[i] }
+func (f byPriority) Less(i, j int) bool { return f[i].Priority > f[j].Priority }
+
+// LoadMetaOptions scans the directory tree for meta-option metadata
+// files and loads them. Any regular file named "option-XX.json" is
+// strictly assumed to be a meta-option file. This strictness allows
+// us to fail hard if such a file cannot be parsed.
+//
+// Loading meta-options from disk allows extra files and their
+// configuration be kept together and defined externally.
+func LoadMetaOptions(dir string) ([]*MetaOption, error) {
+ var meta []*MetaOption
+
+ worker := func(path string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+ if !info.Mode().IsRegular() {
+ return nil
+ }
+ if !strings.HasPrefix(info.Name(), "option-") {
+ return nil
+ }
+ if !strings.HasSuffix(info.Name(), ".json") {
+ return nil
+ }
+
+ content, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
+ }
+
+ var option MetaOption
+ if err := json.Unmarshal(content, &option); err != nil {
+ return fmt.Errorf("failed to parse %s: %v", path, err)
+ }
+
+ log.Printf("Loaded meta-option '%s'", option.Name)
+
+ meta = append(meta, &option)
+ return nil
+ }
+
+ if err := filepath.Walk(dir, worker); err != nil {
+ return nil, err
+ }
+ return meta, nil
+}
+
+func BuildOptions(metaOptions []*MetaOption) *Options {
Review comment:
Yeah, you should be able to just add a `boot_test.go` in the same directory with `package main`, then you can add functions like `Test<function you are testing>(t *testing.T)` which will get automatically picked up by go when you run go test in the directory. Here's an example test file - https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/funcx/fn_test.go
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1071218570
R: @lukecwik
CC: @kennknowles
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075671299
Run Dataflow Runner V2 Java 17 Nexmark Tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832743755
##########
File path: sdks/java/container/common.gradle
##########
@@ -74,6 +74,13 @@ task copyGolangLicenses(type: Copy) {
dependsOn ':release:go-licenses:java:createLicenses'
}
+task copyJdkOptions(type: Copy) {
+ if (imageJavaVersion == "17") {
Review comment:
As a follow up we should do this for jdk 11 as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075905445
https://ci-beam.apache.org/job/beam_PostCommit_Java_Nexmark_DataflowV2_Java17_PR/7/ and https://ci-beam.apache.org/job/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_Java17_PR/6/ passed. GitHub UI failed to update based upon the Jenkins job status.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1074739204
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17110: [WIP] Open modules for jamm
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1069795353
Run Load Tests Java 17 CoGBK Dataflow V2 Batch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832634891
##########
File path: sdks/java/container/boot.go
##########
@@ -193,7 +195,35 @@ func main() {
} else {
args = append(args, jammAgentArgs)
}
+ // Apply meta options
+ const metaDir = "/opt/apache/beam/options"
+ metaOptions, err := LoadMetaOptions(metaDir)
+ javaOptions := BuildOptions(metaOptions)
+ // (1) Add custom jvm arguments: "-server -Xmx1324 -XXfoo .."
+ args = append(args, javaOptions.JavaArguments...)
Review comment:
There may be duplicate args, but there's a priority associated with the MetaOptions, so higher priority will override the lower
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #17110:
URL: https://github.com/apache/beam/pull/17110#issuecomment-1075791714
Run Java PreCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on a change in pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
kileys commented on a change in pull request #17110:
URL: https://github.com/apache/beam/pull/17110#discussion_r832628452
##########
File path: sdks/java/container/boot.go
##########
@@ -211,3 +241,120 @@ func heapSizeLimit(info *fnpb.ProvisionInfo) uint64 {
}
return 1 << 30
}
+
+// Options represents java VM invocation options in a simple,
+// semi-structured way.
+type Options struct {
+ JavaArguments []string `json:"java_arguments,omitempty"`
+ Properties map[string]string `json:"properties,omitempty"`
+ Classpath []string `json:"classpath,omitempty"`
+}
+
+// MetaOption represents a jvm environment transformation or setup
+// that the launcher employs. The aim is to keep the service-side and
+// user-side required configuration simple and minimal, yet allow
+// numerous execution tweaks. Most tweaks are enabled by default and
+// require no input. Some setups, such as Cloud Debugging, are opt-in.
+//
+// Meta-options are usually included with the image and use supporting
+// files, usually jars. A few are intrinsic because they are require
+// additional input or complex computations, such as Cloud Debugging
+// and Cloud Profiling. Meta-options can be enabled or disabled by
+// name. For the most part, the meta-option names are not guaranteed
+// to be backwards compatible or stable. They are rather knobs that
+// can be tuned if some well-intended transformation cause trouble for
+// a customer. For tweaks, the expectation is that the default is
+// almost always correct.
+//
+// Meta-options are simple additive manipulations applied in priority
+// order (applied low to high) to allow jvm customization by adding
+// files, notably enabling customization by later docker layers. The
+// override semantics is prepend for lists and simple overwrite
+// otherwise. A common use case is adding a jar to the beginning of
+// the classpath, such as the shuffle or windmill jni jar, or adding
+// an agent.
+type MetaOption struct {
+ Name string `json:"name,omitempty"`
+ Description string `json:"description,omitempty"`
+ Enabled bool `json:"enabled,omitempty"`
+ Priority int `json:"priority,omitempty"`
+ Options Options `json:"options"`
+}
+
+// byPriority sorts MetaOptions by priority, highest first.
+type byPriority []*MetaOption
+
+func (f byPriority) Len() int { return len(f) }
+func (f byPriority) Swap(i, j int) { f[i], f[j] = f[j], f[i] }
+func (f byPriority) Less(i, j int) bool { return f[i].Priority > f[j].Priority }
+
+// LoadMetaOptions scans the directory tree for meta-option metadata
+// files and loads them. Any regular file named "option-XX.json" is
+// strictly assumed to be a meta-option file. This strictness allows
+// us to fail hard if such a file cannot be parsed.
+//
+// Loading meta-options from disk allows extra files and their
+// configuration be kept together and defined externally.
+func LoadMetaOptions(dir string) ([]*MetaOption, error) {
+ var meta []*MetaOption
+
+ worker := func(path string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+ if !info.Mode().IsRegular() {
+ return nil
+ }
+ if !strings.HasPrefix(info.Name(), "option-") {
+ return nil
+ }
+ if !strings.HasSuffix(info.Name(), ".json") {
+ return nil
+ }
+
+ content, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
+ }
+
+ var option MetaOption
+ if err := json.Unmarshal(content, &option); err != nil {
+ return fmt.Errorf("failed to parse %s: %v", path, err)
+ }
+
+ log.Printf("Loaded meta-option '%s'", option.Name)
+
+ meta = append(meta, &option)
+ return nil
+ }
+
+ if err := filepath.Walk(dir, worker); err != nil {
+ return nil, err
+ }
+ return meta, nil
+}
+
+func BuildOptions(metaOptions []*MetaOption) *Options {
Review comment:
I'm not familiar with the go test framework. Is there an easy way to set that up for the boot script in the Java directory?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik merged pull request #17110: [BEAM-13695] Add option to add-open flags for Java container
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #17110:
URL: https://github.com/apache/beam/pull/17110
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org