[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-404380006 Thanks a lot @rabbah and @tysonnorris for reviewing this big PR ✨. Opened #3872 to track documentation related work This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-400550610 Needs a PG run This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-400304718 I am working on a separate PR which would add new stages to run full test suite with Cosmos. That would allow complete test coverage against this new store. Would raise it once this PR is merged This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-391595062 @tysonnorris I agree current ref count based shutdown handling is not desirable and we should relook into how `ArtifactStore` are closed. May be we deal with this in a separate issue and use existing approach in this PR for now? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-391218477 > On this last one, I think it is a defect of WhiskStore that multiple calls to WhiskEntityStore.datastore() will return multiple unique instances One benefit of this approach is that future test using `MemoryArtifactStore` can work without any side effect of other tests. Caching the result of call of `makeStore` would make `shutdown` handling tricky. Currently each eventual owner of store instance is responsible for shutdown. Probably in that case we need to have a shutdown support at provider level This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-384567083 ## Indexing Policy CosmosDB provides rich [indexing options][1] which support * Indexing mode - consistent or lazy * Hash Index - For equality check * Range Index - For range check and order by * Control Precision - Allows to make trade-offs between index storage overhead and query performance * Boolean properties are indexed by default - As per [create document api][3] _Booleans and nulls are automatically indexed_ (See dataType) Currently the view based indexes in CouchDB are eventually consistent and all the test around queries account for that. Based on usage following indexes are configured ### Subjects * /uuid/? - Authentication query * /subject/? * /namespaces/[]/uuid/? - Authentication query * /namespaces/[]/name/? * /concurrentInvocations/? - For blacklist check query * /invocationsPerMinute/? - For blacklist check query Key points * Indexing mode is set to consistent * All indexes have precision set to -1 for now. Later based on metrics we can determine if this needs to be tweaked * `key` is not being indexed - Authentication query is performed on uuid and key. Of this index is only created for `uuid` and `key` check should be taken care by filtering in query evaluation. As `uuid` is very much unique number of rows scanned for filtering should be small * `concurrentInvocations` and `invocationsPerMinute` are used in "namespace name/limits" documents which are then used to find out blacklisted namespace if either of them is zero. We can possibly remove this by setting `blocked` to true even for limits docs ### Whisks * /entityType/? - Hash * /namespace/? - Hash * /\_c/rootns/? - Hash * /updated/? - Range Key points * Indexing mode set to consistent ### Activations * /namespace/? * /_c/nspath/? * /start/? Key points * Indexing mode set to lazy - For activations the rate of writes would be much higher compared to those on `whisks` and `subjects`. So here it would be better to keep indexing mode lazy ### Index policy updates Current logic upon system start would query for existing indexes configured on the collection. If there is a mismatch between what is expected and what is currently found it would provision the new indexes. CosmosDB currently shows a strange behavior where it is [creating extra indexes][4] for same path. For e.g. `/namespace/?` is configured with hash index. But post creation if indexes are checked its found that it has both `Range` and `Hash` index configured. To account for these extra indexes in index check this PR uses a custom `IndexingPolicy` type which accounts for this type of behaviour while check if indexes are same. CosmosDB supports [online index policy changes][5] where by change in index definition would be performed asynchronously and online allowing the collection to remain available for writes while transformation is in progress. However if the application makes query relying on new indexes then those queries would fail untill indexing completes based on new index definition. So in future if any change in index definition is done then it should be provisioned prior to deploying new code relying on that index. Current approach of checking for index definition would ensure that index definitions are consistent [1]: https://docs.microsoft.com/en-us/azure/cosmos-db/indexing-policies [2]: https://docs.microsoft.com/en-us/azure/cosmos-db/indexing-policies#index-precision [3]: https://docs.microsoft.com/en-us/rest/api/cosmos-db/create-a-collection [4]: https://stackoverflow.com/questions/43775750/documentdb-creating-extra-indexes [5]: https://azure.microsoft.com/en-in/blog/update-your-documentdb-indexing-policies-online/ This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-383820178 ### Test suite run result In a [separate branch][1] travis is configured to run whole OW test suite as being run currently for master with default `ArtifactStoreProvider` switched to `CosmosDBArtifactStoreProvider`. In such a run [some test got failed][2]. Below is the analysis for them * `WskRestEntitlementTests` - These test make use of `wskadmin` to provision a custom user. Currently `wskadmin` does not support CosmosDB. Hence for now test being ignored * `ActionLimitsTests` - Some of the test here got failed due to Azure/azure-cosmosdb-java#33. PR raised for that. For now being ignored * `SequenceMigrationTests` - These are passing now [1]: https://github.com/chetanmeh/incubator-openwhisk/tree/artifact-store-cosmos-ci [2]: https://scans.gradle.com/s/nftphfau7o4a6/tests/failed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-383820178 ### Test suite run result In a [separate branch][1] travis is configured to run whole OW test suite as being run currently for master with default `ArtifactStoreProvider` switched to `CosmosDBArtifactStoreProvider`. In such a run [some test got failed][2]. Below is the analysis for them * `WskRestEntitlementTests` - These test make use of `wskadmin` to provision a custom user. Currently `wskadmin` does not support CosmosDB. Hence for now test being ignored * `ActionLimitsTests` - Some of the test here got failed due to Azure/azure-cosmosdb-java#33. PR raised for that. For now being ignored * `SequenceMigrationTests` - These are passing now [1]: https://github.com/chetanmeh/incubator-openwhisk/tree/artifact-store-cosmos-ci [2]: https://scans.gradle.com/s/nftphfau7o4a6/tests/failed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB
chetanmeh commented on issue #3562: ArtifactStore implementation for CosmosDB URL: https://github.com/apache/incubator-openwhisk/pull/3562#issuecomment-383820178 ### Test suite run result In a [separate branch][1] travis is configured to run whole OW test suite as being run currently for master with default `ArtifactStoreProvider` switched to `CosmosDBArtifactStoreProvider`. In such a run [some test got failed][2]. Below is the analysis for them * `WskRestEntitlementTests` - These test make use of `wskadmin` to provision a custom user. Currently `wskadmin` does not support CosmosDB. Hence for now test being ignored * `ActionLimitsTests` - Some of the test here got failed due to Azure/azure-cosmosdb-java#33. PR raised for that. For now being ignored * `SequenceMigrationTests` - These are passing now [1]: https://github.com/chetanmeh/incubator-openwhisk/tree/artifact-store-cosmos-ci [2]: https://scans.gradle.com/s/nftphfau7o4a6/tests/failed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
