Skip to content

Conversation

@rnewson
Copy link
Member

@rnewson rnewson commented Mar 10, 2021

Overview

Verify correct behaviour when a database is updated concurrently, particularly for secondary indexes.

Testing recommendations

N/A, these are just new tests.

Related Issues or Pull Requests

Motivated by #3382

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

Copy link
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it passes:

make elixir tests=test/elixir/test/concurrent_writes_test.exs
...
ConcurrentWritesTest
  * test Primary data tests (1001.8ms)
  * test Secondary data tests (5860.7ms)

I tried reverting ec4b213 and it still passed. Ideally, we'd want it to fail then. If you want let's merge this tests but if possible see if we can make it fail with another more complex test. I think the key to failure there was to have few view queries, such that he view would be forced to update a few times perhaps.

Copy link
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

test "Primary data tests", context do
db_name = context[:db_name]
parent = self()
Enum.each(1..99,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: make 99 a variable at the top somewhere so we could make the test run with say 1000 or more docs if needed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can also assert that doc puts succeeded by checking

   r = Couch.put("/#{db_name}/doc#{x}", body: %{:a => x})
   assert r.status_code == 201

@rnewson rnewson force-pushed the concurrent_write_tests branch from 6de3cac to c1c8295 Compare March 10, 2021 17:33
@rnewson
Copy link
Member Author

rnewson commented Mar 10, 2021

Added nick's suggestions and his max_sessions work.

@rnewson rnewson merged commit 82bd4d4 into main Mar 10, 2021
@rnewson rnewson deleted the concurrent_write_tests branch March 10, 2021 18:23
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.

2 participants