Skip to content

finish/failed uncompleted transaction#5138

Merged
style95 merged 1 commit into
apache:masterfrom
jiangpengcheng:fix_uncompleted_transaction
Sep 30, 2021
Merged

finish/failed uncompleted transaction#5138
style95 merged 1 commit into
apache:masterfrom
jiangpengcheng:fix_uncompleted_transaction

Conversation

@jiangpengcheng

@jiangpengcheng jiangpengcheng commented Jun 29, 2021

Copy link
Copy Markdown
Contributor

Just find that in some cases, ElasticSearchActivationStore doesn't complete transaction.

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
  • Scheduler
  • 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.

@ningyougang

ningyougang commented Jun 29, 2021

Copy link
Copy Markdown
Contributor

LGTM

@codecov-commenter

codecov-commenter commented Jun 29, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5138 (7c79078) into master (eacbe1a) will increase coverage by 29.98%.
The diff coverage is 22.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #5138       +/-   ##
===========================================
+ Coverage   45.26%   75.25%   +29.98%     
===========================================
  Files         234      234               
  Lines       13382    13389        +7     
  Branches      541      551       +10     
===========================================
+ Hits         6058    10076     +4018     
+ Misses       7324     3313     -4011
Impacted Files Coverage Δ
...e/elasticsearch/ElasticSearchActivationStore.scala 81.76% <22.22%> (+21.02%) ⬆️
...whisk/connector/kafka/KafkaProducerConnector.scala 80.00% <0.00%> (-7.50%) ⬇️
...ntainerpool/v2/FunctionPullingContainerProxy.scala 78.12% <0.00%> (+0.34%) ⬆️
.../org/apache/openwhisk/core/entity/EntityPath.scala 100.00% <0.00%> (+1.88%) ⬆️
...la/org/apache/openwhisk/common/TransactionId.scala 94.38% <0.00%> (+2.24%) ⬆️
.../org/apache/openwhisk/http/PoolingRestClient.scala 90.90% <0.00%> (+3.03%) ⬆️
...sk/core/containerpool/docker/DockerContainer.scala 95.60% <0.00%> (+3.29%) ⬆️
...in/scala/org/apache/openwhisk/common/Logging.scala 78.99% <0.00%> (+3.65%) ⬆️
...tainerpool/docker/DockerClientWithFileAccess.scala 96.00% <0.00%> (+4.00%) ⬆️
...a/org/apache/openwhisk/core/entity/TimeLimit.scala 95.65% <0.00%> (+4.34%) ⬆️
... and 126 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 eacbe1a...7c79078. Read the comment docs.

rabbah
rabbah reviewed Jul 6, 2021
View reviewed changes

@rabbah rabbah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@dgrove-oss

dgrove-oss commented Sep 29, 2021

Copy link
Copy Markdown
Member

Should this be merged?

@jiangpengcheng

jiangpengcheng commented Sep 30, 2021

Copy link
Copy Markdown
Contributor Author

oh, I will rebase master branch first to ensure there is no error

@jiangpengcheng jiangpengcheng force-pushed the fix_uncompleted_transaction branch from f868951 to 7c79078 Compare September 30, 2021 02:20
@style95 style95 merged commit 3753daf into apache:master Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rabbah rabbah rabbah approved these changes
@ningyougang ningyougang ningyougang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jiangpengcheng @ningyougang @codecov-commenter @dgrove-oss @rabbah @style95