-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Test top 3 APIs on GAE Flex, GKE, and GCP #1752
Comments
Removing bigquery. |
@ludoch FYI |
Re GAE Java 8 Standard: "the biggest instance on GAE Std has 1Gb of ram, which may be not enough for any practical usage of google-cloud-java": I hope that doesn't mean that making an API call requires 1.5Gb of RAM. Maybe the issue is with the test that has high parallelism? Can you run everything sequentially? Leaving the memory issue aside for a second, I would start with fixing the issues that are more clearly tied to the difference in environments. Here's two of them: Issue 3 ("env variable"): Can you include the parent exception where you throw that RuntimeException? The API classes should always be loadable. Minor points re the message: "com.google.appengine.runtime.version" is a system property, not an environment variable; the preferred way to check for GAE presence is to check that the "com.google.appengine.runtime.environment" system property is present; it's also a good idea to log its value in case of error; we don't call the AppEngine APIs "appengine-sdk" classes -- "AppEngine API classes" sounds better. Issue 5 ("thread manager"): "Can't make API call urlfetch.Fetch in a thread that is neither the original request thread nor a thread created by ThreadManager" is fully expected if you're using a ThreadPoolExecutor with the default JRE ThreadFactory. Instead, you should pass in a ThreadFactory returned by https://cloud.google.com/appengine/docs/standard/java/javadoc/com/google/appengine/api/ThreadManager. If your test app is not a frontend, as I'd expect given that the tests may take more than 60s to complete, then you should ThreadManager.backgroundThreadFactory(). |
Hi Roberto,
The issue 3 is clearly veneer's problem and must be easy to fix. Also sorry
for calling it env variable, I don't know why I did that (probably I did it
automatically, thinking about a similar issue
#1492, which
uses both: system properties and env variables).
Issue 5. According to gblock@ there should be zero cost to supporting GAE
Standard on 8+, and code should just work. Not being able to use normal
thread executors (without specific thread factories based on env) is a *big
issue* for us. We do use them in many places, it is also a part of GAPIC
clients generation. In general there is a lot of heavy multithreading in
multiple places of the library, which is hard to support by itself.
Customization for GAE will make things much worse. Plus, we don't want to
have dependency on AppEngine API in libraries at all (ThreadManger belongs
to the appengine api).
Also, actually the code I was running was already relying on default thread
factory heavily. The tests themselves are executed in a background thread,
not withing the context of a HTTP request (the app receives POST request,
submits the task in default ThreadPoolExecutor, and returns). All that
worked just fine, except that single case, described in the issue.
Not, that I was not able to run integration tests on GAEJ8 yet, because of
the issue 3 (prevents ITs from starting). After that issue is fixed, there
are potentially more problems with GAE std J8.
Thanks
…On Fri, Apr 7, 2017 at 12:56 PM, Roberto Chinnici ***@***.***> wrote:
Re GAE Java 8 Standard:
"the biggest instance on GAE Std has 1Gb of ram, which may be not enough
for any practical usage of google-cloud-java": I hope that doesn't mean
that making an API call requires 1.5Gb of RAM. Maybe the issue is with the
test that has high parallelism? Can you run everything sequentially?
Leaving the memory issue aside for a second, I would start with fixing the
issues that are more clearly tied to the difference in environments. Here's
two of them:
Issue 3 ("env variable"): Can you include the parent exception where you
throw that RuntimeException? The API classes should always be loadable.
Minor points re the message: "com.google.appengine.runtime.version" is a
system property, not an environment variable; the preferred way to check
for GAE presence is to check that the "com.google.appengine.runtime.environment"
system property is present; it's also a good idea to log its value in case
of error; we don't call the AppEngine APIs "appengine-sdk" classes --
"AppEngine API classes" sounds better.
Issue 5 ("thread manager"): "Can't make API call urlfetch.Fetch in a
thread that is neither the original request thread nor a thread created by
ThreadManager" is fully expected if you're using a ThreadPoolExecutor with
the default JRE ThreadFactory. Instead, you should pass in a ThreadFactory
returned by https://cloud.google.com/appengine/docs/standard/java/
javadoc/com/google/appengine/api/ThreadManager. If your test app is not a
frontend, as I'd expect given that the tests may take more than 60s to
complete, then you should ThreadManager.backgroundThreadFactory().
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#1752 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AYI4w3lvGBcKCrM5Px--lhzpSK-kA4q5ks5rtpTqgaJpZM4MdPkG>
.
|
Could you explain which App Engine APIs you need to call? If we brought their number down to zero, then you could run the client code in any thread. |
We don't call any App Engine APIs, we don't even depend on App Engine Api explicitly. I thought that your previous comment for the issue 5 implied that we should switch to specific ThreadFactory when we create threads (thus we would have to add dependency on App Engine API). Currently we do not use it. google-cloud-java currently does not depend on App Engine API at all, but somehow we receive app engine api specific error, when running on GAE Std J8. This error (issue 5) is not present on any other environment. On all platforms I was running 100% same code, without any modifications, except those configuration changes, which are env specific, but as for Java code itself it is 100% same everywhere. Note, there could be some wierd transitive dependency on appengine-sdk, but I cannot find any. The only place where we use appengine-sdk as dependency is for google-cloud-appengine-flex-compat module, but it is a requirement fro java-compat environment, and is not related to GAE Std Java8 tests. |
Are you using java.net.HttpURLConnection by any chance? |
By default we redirect HttpURLConnection to the URLFetch API. This can overridden by adding this line to application-web.xml: You should add it, but I think that we have a bug in Java 8 and that setting is not honored anyway. It'd be nice to verify this. If you can log the stack trace from the IOException where you use HttpURLConnection, I'd expect it to show some urlfetch API classes even when url-stream-handler is set to native. |
Correct line: |
It seems having no impact. https://std-java8-dot-vam-veneer.appspot.com/ is currently deployed with the following appengine-web.xml:
Running a test for |
Thanks. It looks like we have to fix some bugs on our side before the metadata server can work. In the meantime, could you set the "GOOGLE_CLOUD_PROJECT" system property in application-web.xml? Based on the link to the code you provided before, if the system property is set the client won't even try to use HttpURLConnection to retrieve the metadata. |
Sorry, I keep writing the descriptor name wrong. It's
|
That did not have impact on the issue 5, but it kind of fixes issue 3, and allows to run integration tests, with lots of NoClassDefFound, for example:
|
That's odd, because the API classes should always be available. Does the test infrastructure create any classloaders? Could you print out the classloader for one of the test classes, like ITDatastoreTest? If it's a URLClassLoader, please also print the result of getURLs(). |
Here it is:
|
If I add the sdk explicitly in app's pom as dependency, then I get another set of errors:
|
Right, adding the GAE SDK API jar in the pom dependency is the temporary solution to address Soon, a new Java8 GAE Standard runtime will make this addition irrelevant, but this is what you need to do for now. |
* updating 'PubSubExample' to latest api (#1808) * updating pubsub sample to latest api * Pubsub update (#1818) * Update GAPIC layer * Manual updates to support pubsub changes * Update spi classes (#1817) * Update README.md (#1825) [ci skip] * Add DURABLE_REDUCED_AVAILABILITY storage class (#1834) Otherwise we crash when servers return this value. * logging: make flush wait for writes (#1815) This PR still isn't completely correct, since it does not force any RPC to immediately be issued. However, flush should now correctly wait for RPCs representing prior calls to publish to complete and any failures to be reported to ErrorManager before returning. * use new PartitionKey implementation (#1841) bumping gax version to 0.8.0 for the new implementation. reflects googleapis/gapic-generator#1153 . * Release 0.11.0 * Updating version in README files. [ci skip] * Update version to 0.11.1-SNAPSHOT (#1843) * Added a missing @test annotation (#1842) * pubsub: make Subscriber use ApiService (#1824) Fixes #1761. * SPI: Adding @experimentalapi back to logging client classes (#1844) * Bumping NL, Translate to beta (#1848) * Release 0.11.1 * Updating version in README files. [ci skip] * Update version to 0.11.2-SNAPSHOT (#1852) * pubsub: remove obsolete doc references (#1823) This PR removes references to the deprecated code of the docs. The emulator section is rewritten. Fixes #1789. * adding functions to manage life cycle of resources in ITComputeTest (#1768) *adding functions to ITComputeTest, in order to make sure resources created during tests can be properly deleted even if tests fail or become timed out. * refactor and incorporate feedbacks * implement `add` function with compile-time type checking * add `remove` method to remove a resource from managed resources * use Id's as handles to resources * fix copyright header * rename class name * modify remove function, pass delete function to each add method * address comments * Add Speech v1. (#1858) * Make logging overrides the default channel provider (#1820) * Added more unit tests for SessionPool (#1862) * Rename Translate title to Translation [ci skip] (#1867) * Release 0.11.2 * Updating version in README files. [ci skip] * Update pom.xml version to 0.11.3-SNAPSHOT (#1870) * pubsub: acquire FlowController before releasing (#1831) * Revert "pubsub: acquire FlowController before releasing (#1831)" (#1872) This reverts commit 3717ac6. This change brings up another serious bug. If the number of messages we pull in one RPC is greater than the number size of the semaphore, we deadlock forever. Will redo this later. * pubsub: make deprecated methods package-private (#1861) add back mistakenly deleted test Fixes #1828. * pubsub: rename newBuilder to defaultBuilder (#1873) Fixes #1853. * GAE (Flex_Java/Flex_Custom/Flex_Compat/Std_Java8), GCE, GKE testing app for gcj (#1859) Appengine tests for #1752 * Replace a constant of type Set with ImmutableSet (#1876) * Language v1beta2 Release (#1878) * Language v1beta2 Release * Language v1 update * Regenerating SPI: use setEndpoint (#1879) Reflects googleapis/gapic-generator#1172. Updates #1835. * remove last use of setPort/setServiceAddress (#1880) Fixes #1835. * new code snippet for push subscription + cleanup of deprecated snippets (#1875) * cleaning up PubSubExample adding snippet for creating a subscription with a push endpoint * updating start, end tags for snippets, adding async pull snippet tag * Release 0.12.0 * Updating version in README files. [ci skip] * Update version to 0.12.1-SNAPSHOT (#1886) * Update version of google-auth-java to 0.6.1 (#1888) * Update version of google-auth-java to 0.6.1 Latest version of google-auth-java contains a fix for auth token refresh failures. * Remove harcoded auth dependencies * Don't use `UrlFetchTransport` in App Engine Flex environment (#1893) * Don't use `UrlFetchTransport` in App Engine Flex environment #1492 * Add annotations to specify GCP launch stage (#1889) The Google Cloud Platform launch stage (https://cloud.google.com/terms/launch-stages) is a signifier of the level of access and support that can be expected of a particular feature. These annotations will be used to clearly demarcate features as being in a state other than General Availability to help set user expectations accordingly. * Add Identity Access Management (IAM) to the Storage API (#1812) Adds support for bucket-level IAM (currently in limited alpha). More information about IAM in Google Cloud Storage can be found at https://cloud.google.com/storage/docs/access-control/iam
…mplates/java_library/.kokoro (#1752) (#1079) * build(deps): bump protobuf from 3.20.1 to 3.20.2 in /synthtool/gcp/templates/java_library/.kokoro (#1752) build(deps): bump protobuf Bumps [protobuf](https://github.com/protocolbuffers/protobuf) from 3.20.1 to 3.20.2. - [Release notes](https://github.com/protocolbuffers/protobuf/releases) - [Changelog](https://github.com/protocolbuffers/protobuf/blob/main/generate_changelog.py) - [Commits](protocolbuffers/protobuf@v3.20.1...v3.20.2) --- updated-dependencies: - dependency-name: protobuf dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Source-Link: googleapis/synthtool@239f962 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:d5da32501662e4e53365220cc14cfb1d3b9446585397d57dac50171d92556ae7 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alice <65933803+alicejli@users.noreply.github.com>
Test Storage, Datastore, and Logging client libraries on all the GCP platforms (don't include GAE Standard).
Close this issue once we confirm it all works.
The text was updated successfully, but these errors were encountered: