You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2022/11/29 08:46:06 UTC
[GitHub] [openwhisk] bdoyle0182 opened a new pull request, #5355: Fix missing attachment stuck actions
bdoyle0182 opened a new pull request, #5355:
URL: https://github.com/apache/openwhisk/pull/5355
## Description
This is a critical bug that's gone unsolved for several years. Since attachments are considered secondary files to the main action document, if the attachment is lost for whatever reason (upload timeout, concurrent upload, bad database replication, etc.) the action document is therefore corrupted in a stuck state because the document factory is set up to do a get of the document first when performing a `putEntity` or `deleteEntity`. The `WhiskAction` type overrides the `get` operation to include getting the attachment as a part of the `get` operation. However once the root document is retrieved, the attachment is then tried to be read and returns a not found making the `get` return a `DocumentNotFoundException` to the `putEntity` or `deleteEntity`. You can therefore never delete or update an action (that is broken since the attachment was lost) once in this state through the openwhisk api and must go through an openwhisk admin to delete the document directly from the artifact db.
Worse, for updating an action it incorrectly returns a 409 to the user when a conflict isn't what's actually happening in this case. The `get` of the `WhiskAction` correctly returns a `DocumentNotFoundException` when the attachment is not found, however the `putEntity` function then catches this and tries to put the document without a revision thinking it is a brand new document for the artifact db when the root document does in fact exist so the db returns a 409 when a put request is sent to it without a revision; which is what is then fed back to the user on any update action attempt.
I have implemented a simple fix that finally resolves this issue for updating and deleting an action in this missing attachment state without impacting any other behavior of the system outside of the update and delete action api. The change is simple in that we recover in the `get` of the `WhiskAction` type if getting the attachment fails for not being found only if an ignore boolean is set. This ignore boolean is only set when `get` is called within `putEntity` and `getEntity`, and only has an effect on the `WhiskAction` type.
I've reproduced and verified in my environment that this change resolves the issue.
## Related issue and scope
- [X] I opened an issue to propose and discuss this change (#651)
## My changes affect the following components
- [ ] API
- [X] 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
- [X] 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:
- [X] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [X] I reviewed the [style guides](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md#coding-standards) 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.
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] codecov-commenter commented on pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #5355:
URL: https://github.com/apache/openwhisk/pull/5355#issuecomment-1340218934
# [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5355?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#5355](https://codecov.io/gh/apache/openwhisk/pull/5355?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7337d78) into [master](https://codecov.io/gh/apache/openwhisk/commit/85788875d597f4225909479cefbdf403c0d19ff2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8578887) will **decrease** coverage by `15.18%`.
> The diff coverage is `57.14%`.
```diff
@@ Coverage Diff @@
## master #5355 +/- ##
===========================================
- Coverage 81.46% 66.28% -15.19%
===========================================
Files 240 240
Lines 14407 14407
Branches 618 601 -17
===========================================
- Hits 11736 9549 -2187
- Misses 2671 4858 +2187
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5355?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ache/openwhisk/core/database/DocumentFactory.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvRG9jdW1lbnRGYWN0b3J5LnNjYWxh) | `100.00% <ø> (ø)` | |
| [.../openwhisk/core/scheduler/queue/QueueManager.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9zY2hlZHVsZXIvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9vcGVud2hpc2svY29yZS9zY2hlZHVsZXIvcXVldWUvUXVldWVNYW5hZ2VyLnNjYWxh) | `6.89% <0.00%> (-74.61%)` | :arrow_down: |
| [...ntainerpool/v2/FunctionPullingContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC92Mi9GdW5jdGlvblB1bGxpbmdDb250YWluZXJQcm94eS5zY2FsYQ==) | `78.50% <50.00%> (ø)` | |
| [...org/apache/openwhisk/core/entity/WhiskAction.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L1doaXNrQWN0aW9uLnNjYWxh) | `92.34% <100.00%> (-1.10%)` | :arrow_down: |
| [...rg/apache/openwhisk/core/controller/ApiUtils.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9BcGlVdGlscy5zY2FsYQ==) | `64.95% <100.00%> (ø)` | |
| [...scala/org/apache/openwhisk/common/time/Clock.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90aW1lL0Nsb2NrLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nwhisk/core/scheduler/queue/ContainerCounter.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9zY2hlZHVsZXIvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9vcGVud2hpc2svY29yZS9zY2hlZHVsZXIvcXVldWUvQ29udGFpbmVyQ291bnRlci5zY2FsYQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...hisk/core/scheduler/message/ContainerMessage.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9zY2hlZHVsZXIvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9vcGVud2hpc2svY29yZS9zY2hlZHVsZXIvbWVzc2FnZS9Db250YWluZXJNZXNzYWdlLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [52 more](https://codecov.io/gh/apache/openwhisk/pull/5355/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
URL: https://github.com/apache/openwhisk/pull/5355
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
URL: https://github.com/apache/openwhisk/pull/5355
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 commented on pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 commented on PR #5355:
URL: https://github.com/apache/openwhisk/pull/5355#issuecomment-1330287330
Line 336 of `ApiUtils` is the culprit of the 409 where it will catch and try to write the document as if it were new without revision
` case _: NoDocumentException =>
logging.debug(this, s"[PUT] entity does not exist, will try to create it")
create().map(newDoc => (None, newDoc))`
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
URL: https://github.com/apache/openwhisk/pull/5355
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 closed pull request #5355: Fix missing attachment stuck actions
URL: https://github.com/apache/openwhisk/pull/5355
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 merged pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 merged PR #5355:
URL: https://github.com/apache/openwhisk/pull/5355
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 commented on pull request #5355: Fix missing attachment stuck actions
Posted by GitBox <gi...@apache.org>.
bdoyle0182 commented on PR #5355:
URL: https://github.com/apache/openwhisk/pull/5355#issuecomment-1330287993
@rabbah you might be interested in this bug since you were on the original issue
--
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: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org