Switch to Cosmos SDK v3 for Cache Invalidator Service #4677

chetanmeh · 2019-10-09T18:42:07Z

Switches to Cosmos SDK v3 which is the next supported SDK for using ChangeFeedProcessor support

Closes #4663

Related issue and scope

I opened an issue to propose and discuss this change (Use CosmosDB v3 SDK for Cache Invalidator Service #4663)

My changes affect the following components

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.

chetanmeh · 2019-10-09T19:07:27Z

...or/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/cache/ChangeFeedConsumer.scala

+      .feedContainer(targetContainer)
+      .leaseContainer(leaseContainer)
+      .options(feedOpts)
+      .asInstanceOf[ChangeFeedProcessorBuilderImpl] //observerFactory is not exposed hence need to cast to impl


Opened Azure/azure-sdk-for-java#5744 to expose the proper api so that we do not have to rely on implementation details

codecov-io · 2019-10-14T14:15:10Z

Codecov Report

Merging #4677 into master will decrease coverage by 4.7%.
The diff coverage is 12.19%.

@@            Coverage Diff             @@
##           master    #4677      +/-   ##
==========================================
- Coverage   83.05%   78.34%   -4.71%     
==========================================
  Files         189      196       +7     
  Lines        8555     8803     +248     
  Branches      597      603       +6     
==========================================
- Hits         7105     6897     -208     
- Misses       1450     1906     +456

Impacted Files	Coverage Δ
...ore/database/cosmosdb/cache/CacheInvalidator.scala	`0% <0%> (-100%)`	⬇️
...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala	`0% <0%> (-94.74%)`	⬇️
.../openwhisk/core/database/cosmosdb/cache/Main.scala	`0% <0%> (ø)`	⬆️
...e/database/cosmosdb/cache/KafkaEventProducer.scala	`0% <0%> (-76.48%)`	⬇️
...e/database/cosmosdb/cache/ChangeFeedConsumer.scala	`0% <0%> (ø)`
...nwhisk/core/database/RemoteCacheInvalidation.scala	`80% <100%> (ø)`	⬆️
...base/cosmosdb/cache/WhiskChangeEventObserver.scala	`50% <28.57%> (-50%)`	⬇️
...core/database/cosmosdb/RxObservableImplicits.scala	`0% <0%> (-100%)`	⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala	`0% <0%> (-96.61%)`	⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala	`0% <0%> (-93.8%)`	⬇️
... and 48 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 00cf75a...6320ec9. Read the comment docs.

chetanmeh · 2019-10-17T07:46:10Z

For some reason the CacheInvalidatorTest is still failing in fork where CosmosDB tests are enabled. So need to validate that

chetanmeh · 2019-10-18T10:50:01Z

Build passed on fork with CosmosDB tests enabled

…Java future support

This seems to be observed only for case where collection to monitor is created fresh (in test setups). Using `startFromBeginning` option seems to fix this

tysonnorris · 2019-10-21T15:47:21Z

...or/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/cache/ChangeFeedConsumer.scala

+import java.util
+
+import akka.Done
+import akka.event.slf4j.SLF4JLogging


Any reason this shouldn't use the same Logging/AkkaLogging classes from common/scala? I guess this applies to all the other classes in cacheinvalidator.

In most cases (except the one to log the progress) the logs were mostly for lifecycle related messages (startup/shutdown) and the CosmosDB SDK also logs using slf4j directly so either of those logging should be fine.

Switched to AkkaLogging now for consistency

tysonnorris · 2019-10-21T15:50:47Z

.../main/scala/org/apache/openwhisk/core/database/cosmosdb/cache/WhiskChangeEventObserver.scala

+      case Success(_) =>
+        MetricEmitter.emitCounterMetric(feedCounter, docs.size)
+        recordLag(context, docs.last)
+    }


should we log something on failures here?

Logging was being done by CosmosDB SDK for failure but added a log here also now such that we can always capture the failure case

tysonnorris

LGTM some minor comments

chetanmeh commented Oct 9, 2019

View reviewed changes

rabbah approved these changes Oct 11, 2019

View reviewed changes

chetanmeh force-pushed the cache-invalidator-v3 branch from 8e900f8 to 6812ae7 Compare October 12, 2019 08:10

chetanmeh requested a review from tysonnorris October 14, 2019 11:56

chetanmeh force-pushed the cache-invalidator-v3 branch 2 times, most recently from a87c462 to b8ee869 Compare October 18, 2019 04:26

chetanmeh added 13 commits October 21, 2019 18:13

Switch to Cosmos SDK v3

c71eb3c

Remove dependency on scala extension for reactor as we can work with …

c6a83d3

…Java future support

Remove logging related logic as v3 uses slf4j and less chatty in logs

bf552d5

Register Kamon metric reporter with Kafka producer

2a7613f

Log the server url upon startup

61c26b1

Refer to topic name from RemoteCacheInvalidation class

a137a35

Remove unused config

61c2cb1

Updated latest released v3 SDK v3.2.2

2ec38ee

Fix compilation issue

d5b73a3

Publish docker image for cache invalidator

10b2f2b

Try increasing the timeout for poll from Kafka topic

c18d69d

At times test fail to read the change

d0d0ea3

This seems to be observed only for case where collection to monitor is created fresh (in test setups). Using `startFromBeginning` option seems to fix this

Update cosmosdb sdk v3 to latest 3.3.0

c460f4f

chetanmeh force-pushed the cache-invalidator-v3 branch from 5b5f919 to c460f4f Compare October 21, 2019 12:43

tysonnorris reviewed Oct 21, 2019

View reviewed changes

tysonnorris approved these changes Oct 21, 2019

View reviewed changes

chetanmeh added 2 commits October 22, 2019 11:59

Switch to Logging from common package instead of direct Slf4j usage

b56c2a6

Warn log for failure

6320ec9

chetanmeh merged commit f69ecb7 into apache:master Oct 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Switch to Cosmos SDK v3 for Cache Invalidator Service #4677

Switch to Cosmos SDK v3 for Cache Invalidator Service #4677

Uh oh!

chetanmeh commented Oct 9, 2019

Uh oh!

chetanmeh Oct 9, 2019

Uh oh!

codecov-io commented Oct 14, 2019 •

edited

Loading

Uh oh!

chetanmeh commented Oct 17, 2019

Uh oh!

chetanmeh commented Oct 18, 2019

Uh oh!

tysonnorris Oct 21, 2019

Uh oh!

chetanmeh Oct 22, 2019

Uh oh!

tysonnorris Oct 21, 2019

Uh oh!

chetanmeh Oct 22, 2019

Uh oh!

tysonnorris left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Switch to Cosmos SDK v3 for Cache Invalidator Service #4677

Switch to Cosmos SDK v3 for Cache Invalidator Service #4677

Uh oh!

Conversation

chetanmeh commented Oct 9, 2019

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

Uh oh!

chetanmeh Oct 9, 2019

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chetanmeh commented Oct 17, 2019

Uh oh!

chetanmeh commented Oct 18, 2019

Uh oh!

tysonnorris Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

chetanmeh Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

tysonnorris Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

chetanmeh Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

tysonnorris left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Oct 14, 2019 •

edited

Loading