Skip to content

Make query limits configurable#4157

Merged
rabbah merged 1 commit intoapache:masterfrom
chetanmeh:query-limit-config
Dec 5, 2018
Merged

Make query limits configurable#4157
rabbah merged 1 commit intoapache:masterfrom
chetanmeh:query-limit-config

Conversation

@chetanmeh
Copy link
Member

@chetanmeh chetanmeh commented Dec 4, 2018

Make limits related to list operation against entities configurable.

Description

Currently the queries related to collections (used while doing list operation) make use of hard coded limits.

Impact due to wsk activation poll

For example if limit=0 in list operation then it uses 200 has the default value. This is used in wsk activation poll which by default generates a GET request like

/api/v1/namespaces/_/activations?docs=true&limit=0&since=1542422386538&skip=0

Currently the poll command once started "probably" sticks to start time and we have seen user leaving such poll running for days (often left in one of the numerous tabs!) which causes the system to pull similar 200 records over and over.

CosmosDB and higher resource usage

Further in our case (using CosmosDB) which does client side merging of query results a sorted query gets executed like below

  1. Initial ask to fetch 200 records
  2. SDK makes call to each partition to fetch query record with limit
    • If limit > default maxItemPerPageCount use maxItemPerPageCount i.e. 100
    • If limit < then use the limit
  3. Perform a merge sort on client side

In such a case at times we have seen for listing 200 records SDK fetch ~ 900 records causing higher resource usage on db side

To reduce impact for such cases I would like to have these limits configurable

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.

@codecov-io
Copy link

Codecov Report

Merging #4157 into master will decrease coverage by 3.41%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4157      +/-   ##
==========================================
- Coverage   84.67%   81.26%   -3.42%     
==========================================
  Files         151      151              
  Lines        7277     7279       +2     
  Branches      466      464       -2     
==========================================
- Hits         6162     5915     -247     
- Misses       1115     1364     +249
Impacted Files Coverage Δ
...apache/openwhisk/core/entitlement/Collection.scala 87.5% <100%> (+0.32%) ⬆️
.../scala/org/apache/openwhisk/core/WhiskConfig.scala 94.26% <100%> (+0.04%) ⬆️
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.54%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-83.34%) ⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala 0% <0%> (-62.5%) ⬇️
...in/scala/org/apache/openwhisk/common/Counter.scala 40% <0%> (-20%) ⬇️
...penwhisk/core/database/cosmosdb/CosmosDBUtil.scala 92% <0%> (-4%) ⬇️
...rg/apache/openwhisk/common/ForcibleSemaphore.scala 88.46% <0%> (-3.85%) ⬇️
... and 25 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 056b5af...f0cc2cd. Read the comment docs.

@rabbah rabbah merged commit ec36051 into apache:master Dec 5, 2018
@chetanmeh
Copy link
Member Author

Opened apache/openwhisk-cli#390 to fix the wsk poll logic to change since time which should also help db as it only has to look into recent results to answer the query

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