Distinguish updates by iilyak · Pull Request #2800 · apache/couchdb

iilyak · 2020-04-21T17:32:31Z

Overview

This PR allow us to distinguish between new doc and update of the existent doc for tracing.

Testing recommendations

update rel/overlay/etc/default.ini as follows

rel/overlay/etc/default.ini
[tracing]
enabled = true ; true | false
app_name = couchdb ; value to use for the `location.application` tag
protocol = http ; udp | http - which reporter to use
endpoint = http://127.0.0.1:14268
 
[tracing.filters]
all = (#{}) -> true

start jaeger in docker container

docker run -d -p 6831:6831/udp -p 16686:16686 -p 14268:14268 jaegertracing/all-in-one:1.14

Start couchdb dev/run --admin=adm:pass --no-join
Create database curl -X PUT -u adm:pass http://127.0.0.1:15984/test
Create document curl -X PUT -u adm:pass http://127.0.0.1:15984/test -H Content-Type:application/json -d '{"_id":"doc1"}'
Update document curl -X PUT -u adm:pass "http://127.0.0.1:15984/test/doc1?rev=8-4f98b201f7a6f147a952fe70651f0a95" -H Content-Type:application/json -d '{"_id":"doc1","val": 4}'
Update document curl -X PUT -u adm:pass "http://127.0.0.1:15984/test/doc1" -H Content-Type:application/json -d '{"_id":"doc1","dfd": 4, "_rev": "11-fdcab8b70c87eb64255f504a5d983ab0"}'
Open http://localhost:16686/ in the browser and search for spans (you should find some traces)
- the first event should have action equal to db.doc.write
- the second event should have action equal to db.doc.update
- the third event should have action equal to db.doc.update

Related Issues or Pull Requests

N/A

Checklist

Code is written and works correctly
Changes are covered by tests
Any new configurable parameters are documented in rel/overlay/etc/default.ini
A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation

…2.0-disable-legacy-checks Upgrade credo to 1.2.0 disable legacy checks

There's a race between the meck:wait call in setup and killing the config_event process. Its possible that we could kill and restart the config_event process after meck:wait returns, but before gen_event:add_sup_handler is called. More likely, we could end up killing the config_event gen_event process before its fully handled the add_sup_handler message and linked the notifier pid. This avoids the race by waiting for config_event to return that it has processed the add_sup_handler message instead of relying on meck:wait for the subscription call.

…st_to_elixir Port form_submit.js test to Elixir

doc: link README-DEV in README

Most of these tests are for quorum and clustered response handling which will no longer exist with FoundationDB. Eventually we'll want to go through these and pick out anything that is still applicable and ensure that we re-add them to the new test suite.

This provides a base implementation of a fabric API backed by FoundationDB. While a lot of functionality is provided there are a number of places that still require work. An incomplete list includes: 1. Document bodies are currently a single key/value 2. Attachments are stored as a range of key/value pairs 3. There is no support for indexing 4. Request size limits are not enforced directly 5. Auth is still backed by a legacy CouchDB database 6. No support for before_doc_update/after_doc_read 7. Various implementation shortcuts need to be expanded for full API support.

This provides a good bit of code coverage for the new implementation. We'll want to expand this to include relevant tests from the previous fabric test suite along with reading through the various other tests and ensuring that we cover the API as deeply as is appropriate for this layer.

This is not an exhaustive port of the entire chttpd API. However, this is enough to support basic CRUD operations far enough that replication works.

This still holds all attachment data in RAM which we'll have to revisit at some point.

When uploading an attachment we hadn't yet flushed data to FoundationDB which caused the md5 to be empty. The `new_revid` algorithm then declared that was because it was an old style attachment and thus our new revision would be a random number. This fix just flushes our attachments earlier in the process of updating a document.

I was accidentally skipping this step around properly serializing/deserializing attachments. Note to self: If someon specifies attachment headers this will likely break when we attempt to pack the value tuple here.

The older chttpd/fabric split configured filters as one step in the coordinator instead of within each RPC worker.

This fixes the behavior when validating a document update that is recreating a previously deleted document. Before this fix we were sending a document body with `"_deleted":true` as the existing document. However, CouchDB behavior expects the previous document passed to VDU's to be `null` in this case.

This was a remnant before we used a version per database.

This changes `chttpd_auth_cache` to use FoundationDB to back the `_users` database including the `before_doc_update` and `after_doc_read` features.

RFC: apache/couchdb-documentation#409 Main API is in the `couch_jobs` module. Additional description of internals is in the README.md file.

Neither partitioned databases or shard splitting will exist in a FoundationDB layer.

This adds the mapping of CouchDB start/end keys and so on to the similar yet slightly different concepts in FoundationDB. The handlers for `_all_dbs` and `_all_docs` have been udpated to use this new logic.

Usually we indicate the transaction status of a Db handle by naming it `TxDb`. This updates fabric2_index:build_indices/2 to match that pattern. Co-Authored-By: Nick Vatamaniuc <vatamane@apache.org>

Functions are easier to read and process if they're defined in the order that they are referenced. Co-Authored-By: Nick Vatamaniuc <vatamane@apache.org>

Each registered index type can now get a signal on when to clean up their indexes.

If a client notices that a job has failed we restart it. If a job failed for a different design document id then we resubmit the build request.

Add tests for view cleanup.

This change allows creation of local `src/couch/rebar.config` and `rebar.config` files to set additional configuration options. This is useful for: - disabling deprecation warnings `{nowarn_deprecated_function, MFAs}` - control debugging in eunit tests - `DEBUG` - `{eunit_compile_opts, [{d, DEBUG, true}]}` - `NODEBUG` - `{eunit_compile_opts, [{d, NODEBUG, true}]}`

Integrate emilio - erang linter Merging it on the grounds of CI pass and +1 in the original PR.

Currently, the size of binary chunks used for values is fixed at the FDB imposed limit of 100kB, although they recommend using 10KB [1], (also note they subtly change units). This makes that value configurable, allowing e.g. benchmarks to compare performance of runs with varying chunk size. The cost is a ~10µs config lookup penalty each time data needs to be chunked. [1] https://www.foundationdb.org/files/record-layer-paper.pdf

* report changes stats intermittently with boolean market Stats are reported at the end of a request. With changes feeds, sometimes the request can be long or forever. This commit allows stats to be reported intermittently via a configurable time in seconds. The report function can return a boolean whether stats was reported so that a reset may not necessarily be needed.

- Rename `clear_expired_range` to `clear_range_to` - Move `EXPIRING_CACHE` layer prefix into fabric2.hrl - Move primary key setting to just after key & value calculations - Factor out `get_val/2` to lookup a key from FDB and unpack the value - Factor out `prefixes/2` - Factor out `fold_range/5`

When an existing key is inserted with different timestamps, the primary key is the same but the primary value is different from the existing one. Currently, this results in a new expiry key being inserted, but the old one is not deleted and lingers until it is removed by the inexorable advance of time via the `remove_expired` server messages. This checks whether there's already primary key for the inserted key, and if so, cleans up the existing expiry key before proceeding with the insert.

Merge keys from rebar.config

By default, transactions are used to check metadata, and possibly reopen the db, to get a current db handle. However, if a `max_age` option is provided and db handle was checked less than `max_age` milliseconds ago, use properties from that cached handle instead. The main use of this feature be in pluggable authorization handlers where it might be necessary to inspect the security doc multiple times for the same request before a final decision is made. `revs_limit/1` was updated as well, mainly for consistency since it is almost identical to `get_security/1`.

Fix typo in error message

chewbranca · 2020-04-21T18:06:52Z

src/chttpd/src/chttpd_httpd_handlers.erl

-handler_info('PUT', [Db, <<"_design">>, Name | Path], _) ->
-    {'db.design.doc.attachment.write', #{
+handler_info('PUT', [Db, <<"_design">>, Name | Path], Req) ->
+    ActionName = case chttpd:qs_value(Req, "rev") of


So I thought about just checking the rev like this, but the problem here is that for PUT and POST, the rev can be embed'ed in the request body and is not guaranteed to be in the query string. I looked at chttpd:qs_value and that appears to just be looking at the query string params, so I don't think this is currently sufficient to properly tag these as write vs update.

I think _bulk_docs may have similar issues, where you can both add new docs and update existing docs in the same request, and would presumably need to inspect each element of the docs field to know what's actually going on?

@chewbranca updated

@jaydoane This is getting into expensive call territory. It would force us to traverse docs multiple times.

@iilyak I think it's just another example of _bulk_docs being a lousy API, and I agree it's expensive. However, each request in principle can do a lot of work, so the upfront cost might be worth it for correctness? Dunno, tbh.

chewbranca · 2020-04-21T18:35:49Z

src/chttpd/src/chttpd_httpd_handlers.erl

-handler_info('PUT', [Db, <<"_design">>, Name], _) ->
-    {'db.design.doc.write', #{'db.name' => Db, 'design.id' => Name}};
+handler_info('PUT', [Db, <<"_design">>, Name], Req) ->
+    case chttpd:qs_value(Req, "rev") of


Should these (ddoc PUT and POST) be updated as well with has_rev?

missed that

dottorblaster and others added 30 commits February 27, 2020 12:45

Merge pull request apache#2518 from dottorblaster/upgrade-credo-to-1.…

949e496

…2.0-disable-legacy-checks Upgrade credo to 1.2.0 disable legacy checks

Bump to jiffy 1.0.4

65bc5b0

Port form_submit.js test to Elixir

d163648

Mark form_submit JS test as ported in README and inside the test itself

3f76c9f

Merge pull request apache#2608 from dottorblaster/port_form_submit_te…

16ed4b7

…st_to_elixir Port form_submit.js test to Elixir

doc: link README-DEV in README

c6b54d6

Merge pull request apache#2619 from apache/doc/readme-links

0a4afde

doc: link README-DEV in README

feat: add mac ci (apache#2622)

93d5263

Update build system for FoundationDB

fff07ba

Update ddoc_cache to use fabric2

14984e8

Start switching chttpd HTTP endpoints to fabric2

3b8baaf

This is not an exhaustive port of the entire chttpd API. However, this is enough to support basic CRUD operations far enough that replication works.

Remove debug logging

814088c

Implement attachment compression

5dbd566

This still holds all attachment data in RAM which we'll have to revisit at some point.

Fix fabric2_txids:terminate/2

2359dab

Convert attachment info to disk terms correctly

ed567a2

I was accidentally skipping this step around properly serializing/deserializing attachments. Note to self: If someon specifies attachment headers this will likely break when we attempt to pack the value tuple here.

Allow for previously configured filters

d15377c

The older chttpd/fabric split configured filters as one step in the coordinator instead of within each RPC worker.

Database config changes should bump the db version

eaec8ac

This was a remnant before we used a version per database.

Implement _users db authentication

f07ed76

This changes `chttpd_auth_cache` to use FoundationDB to back the `_users` database including the `before_doc_update` and `after_doc_read` features.

Update get security to use fabric2

e372594

Fix arity in changes timeout callback

1797619

Fix exception in cache auth doc update

1269278

CouchDB background jobs

f10b554

RFC: apache/couchdb-documentation#409 Main API is in the `couch_jobs` module. Additional description of internals is in the README.md file.

Remove tests for deprecated features.

65b6fe3

Neither partitioned databases or shard splitting will exist in a FoundationDB layer.

Implement _all_dbs/_all_docs API parameters

c9eee15

This adds the mapping of CouchDB start/end keys and so on to the similar yet slightly different concepts in FoundationDB. The handlers for `_all_dbs` and `_all_docs` have been udpated to use this new logic.

davisp and others added 24 commits April 10, 2020 16:30

Rename variables to indicate transaction state

247b809

Usually we indicate the transaction status of a Db handle by naming it `TxDb`. This updates fabric2_index:build_indices/2 to match that pattern. Co-Authored-By: Nick Vatamaniuc <vatamane@apache.org>

Move process_db/1 to match the logical progression

3c0a017

Functions are easier to read and process if they're defined in the order that they are referenced. Co-Authored-By: Nick Vatamaniuc <vatamane@apache.org>

Update to use fabric2_db:get_design_docs/1

3e1c822

Extend fabric2_index callbacks for index cleanup

7bc9148

Each registered index type can now get a signal on when to clean up their indexes.

Implement couch_views:cleanup_indices/2

e0d0391

Implement _view_cleanup for FoundationDB

4275a49

Optionally cleanup stale indices automatically

7aeb54b

Remove jobs on index cleanup

2e5a556

Remove failed view jobs

30fdef7

If a client notices that a job has failed we restart it. If a job failed for a different design document id then we resubmit the build request.

Implement couch_views_cleanup_test.erl

7575152

Add tests for view cleanup.

Integrate emilio - erang linter

9da549e

Merge pull request apache#2789 from cloudant/fdb-integrate-emilio

6bc6f9c

Integrate emilio - erang linter Merging it on the grounds of CI pass and +1 in the original PR.

Merge pull request apache#2783 from cloudant/merge-rebar-config

a527ad1

Merge keys from rebar.config

Fetch doc in same transaction as _all_doc row

0b8dfa6

Fix typo in error message

45e0c30

Merge pull request apache#2796 from cloudant/fix-typo

8554329

Fix typo in error message

Fix trace tag name for {db}/{doc}

3a1140d

Distinguish between new doc and doc update for tracing

bc383e6

chewbranca reviewed Apr 21, 2020

View reviewed changes

Check _rev in the body

d82df5f

chewbranca reviewed Apr 21, 2020

View reviewed changes

wohali changed the base branch from prototype/fdb-layer to main October 21, 2020 18:12

nickva force-pushed the main branch from e41407e to a1fc807 Compare June 7, 2022 20:15

janl closed this Jul 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distinguish updates#2800

Distinguish updates#2800
iilyak wants to merge 262 commits intoapache:mainfrom
cloudant:distinguish-updates

iilyak commented Apr 21, 2020 •

edited

Loading

Uh oh!

chewbranca Apr 21, 2020

Uh oh!

jaydoane Apr 21, 2020

Uh oh!

iilyak Apr 21, 2020

Uh oh!

iilyak Apr 22, 2020

Uh oh!

jaydoane Apr 22, 2020

Uh oh!

chewbranca Apr 21, 2020

Uh oh!

iilyak Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Conversation

iilyak commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Testing recommendations

Related Issues or Pull Requests

Checklist

Uh oh!

chewbranca Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

jaydoane Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

iilyak Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

iilyak Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

jaydoane Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

chewbranca Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

iilyak Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

iilyak commented Apr 21, 2020 •

edited

Loading