Skip to content

Retry NoHttpResponseException for standalone mode in ApacheContainerClient#4644

Merged
chetanmeh merged 3 commits intoapache:masterfrom
chetanmeh:standalone-apache-client
Sep 25, 2019
Merged

Retry NoHttpResponseException for standalone mode in ApacheContainerClient#4644
chetanmeh merged 3 commits intoapache:masterfrom
chetanmeh:standalone-apache-client

Conversation

@chetanmeh
Copy link
Member

@chetanmeh chetanmeh commented Sep 24, 2019

In local setups on Mac using Standalone OpenWhisk mode sometime few test fail. Also some times some steps in compositions are seen to fail. In all such cases the failure happens due to failure during initialization due to NoHttpResponseException

For some container upon retry the init seems to pass fine. This PR modifies the ApacheBlockingContainerClient logic to enable retry for NoHttpResponseException based on explicit config.

By default NoHttpResponseException would not be retried. However if config is enabled then retry would be performed. For standalone case this config is enabled.

With this change all test now pass on Mac in a reliable way. This is also needed for some conductor related test in #4632

Description

Related issue and scope

  • I opened an issue to propose and discuss this change (#????)

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is guarded by a config parameter which preserves current behavior, LGTM.

# as its not sure that server has processed the request or not
# At times this issue is seen in local setups where with retry the request flow
# work as expected. So for such cases like when running in Standalone mode this
# setting can be enabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would perhaps highlight/stress that this setting should generally be false unless you know what you're doing.

@codecov-io
Copy link

Codecov Report

Merging #4644 into master will decrease coverage by 5.87%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4644      +/-   ##
==========================================
- Coverage   84.62%   78.75%   -5.88%     
==========================================
  Files         183      183              
  Lines        8349     8353       +4     
  Branches      568      577       +9     
==========================================
- Hits         7065     6578     -487     
- Misses       1284     1775     +491
Impacted Files Coverage Δ
.../scala/org/apache/openwhisk/core/WhiskConfig.scala 94.77% <100%> (+0.03%) ⬆️
.../containerpool/ApacheBlockingContainerClient.scala 70.68% <33.33%> (-2.04%) ⬇️
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...ore/database/cosmosdb/cache/CacheInvalidator.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.89%) ⬇️
...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala 0% <0%> (-94.74%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...e/database/cosmosdb/cache/ChangeFeedListener.scala 0% <0%> (-86.67%) ⬇️
...e/database/cosmosdb/cache/KafkaEventProducer.scala 0% <0%> (-76.48%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-74.08%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b536efe...6438ac7. Read the comment docs.

@chetanmeh chetanmeh merged commit 5de865c into apache:master Sep 25, 2019
@chetanmeh chetanmeh deleted the standalone-apache-client branch September 25, 2019 04:26
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
…lient (apache#4644)

Adding a new setting to enable retry of NoHttpResponseException based on configuration. 

This setting is enabled in standalone mode to workaround some Docker network issue where first call of `/init` fails due to this exception. Same init passes upon retry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants