Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow persisted couchdb directory mount. #4250

Merged
merged 1 commit into from
Feb 12, 2019
Merged

Conversation

rabbah
Copy link
Member

@rabbah rabbah commented Feb 6, 2019

For example:
wskdev couchdb -d -e"db_persist_path=/home/couchd/openwhisk"

Also should fix #4236. (fix moved to #4262).

Description

Related issue and scope

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.

@rabbah
Copy link
Member Author

rabbah commented Feb 6, 2019

@style95 I think this is something you might be interested in from PR history --- you tried to do something with this some time back. I could be missing something and would appreciate your insight.

@codecov-io
Copy link

codecov-io commented Feb 6, 2019

Codecov Report

Merging #4250 into master will decrease coverage by 4.73%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4250      +/-   ##
==========================================
- Coverage   85.35%   80.61%   -4.74%     
==========================================
  Files         161      161              
  Lines        7532     7532              
  Branches      498      498              
==========================================
- Hits         6429     6072     -357     
- Misses       1103     1460     +357
Impacted Files Coverage Δ
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.6%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-83.34%) ⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala 4.54% <0%> (-45.46%) ⬇️
...in/scala/org/apache/openwhisk/common/Counter.scala 40% <0%> (-20%) ⬇️
...nwhisk/core/database/cosmosdb/CosmosDBConfig.scala 93.75% <0%> (-6.25%) ⬇️

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 6549b9c...4070fa0. Read the comment docs.

@rabbah rabbah mentioned this pull request Feb 6, 2019
21 tasks
Copy link
Contributor

@cbickel cbickel left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks a lot for this change. I will use it for my local installation instantly :)

@rabbah rabbah added couchdb review Review for this PR has been requested and yet needs to be done. labels Feb 7, 2019
@rabbah rabbah force-pushed the couchdb branch 3 times, most recently from f0b1ed3 to 3389c73 Compare February 8, 2019 18:17
@style95
Copy link
Member

style95 commented Feb 10, 2019

@rabbah Thank you for asking me.

I have deployed CouchDB in a distributed environment with this change.

I think we need to add some logic to skip setting up the cluster in case the cluster is already formed.

If I redeploy CouchDB nodes, it tries to readd the nodes to the cluster.
But they are already added in the membership list as we persist the data directory and the cluster formation is failed.

TASK [couchdb : add remote nodes to the cluster] *******************************************************************************************************************************************************************************************************************************
Sunday 10 February 2019  22:41:53 +0900 (0:00:00.083)       0:00:18.239 *******
fatal: [10.105.65.216]: FAILED! => {"cache_control": "must-revalidate", "changed": false, "connection": "close", "content": "{\"error\":\"conflict\",\"reason\":\"Document update conflict.\"}\n", "content_length": "58", "content_type": "application/json", "date": "Sun, 10 Feb 2019 13:41:54 GMT", "json": {"error": "conflict", "reason": "Document update conflict."}, "msg": "Status code was not [201]: HTTP Error 409: Conflict", "redirected": false, "server": "CouchDB/2.1.2 (Erlang OTP/17)", "status": 409, "url": "http://10.105.65.214:5984/_cluster_setup", "x_couch_request_id": "c7521df26e", "x_couchdb_body_time": "0"}

Status code was not [201]: HTTP Error 409: Conflict
fatal: [10.105.65.215]: FAILED! => {"cache_control": "must-revalidate", "changed": false, "connection": "close", "content": "{\"error\":\"conflict\",\"reason\":\"Document update conflict.\"}\n", "content_length": "58", "content_type": "application/json", "date": "Sun, 10 Feb 2019 13:41:54 GMT", "json": {"error": "conflict", "reason": "Document update conflict."}, "msg": "Status code was not [201]: HTTP Error 409: Conflict", "redirected": false, "server": "CouchDB/2.1.2 (Erlang OTP/17)", "status": 409, "url": "http://10.105.65.214:5984/_cluster_setup", "x_couch_request_id": "da0d5465ff", "x_couchdb_body_time": "0"}

Status code was not [201]: HTTP Error 409: Conflict

As I mentioned above, one option could be to skip the clustering setup in case cluster is already enabled like this:

- name: check whether the cluster is setup or not
  uri:
    url: "{{ db.protocol }}://{{ ansible_host }}:{{ db.port }}/_cluster_setup"
    method: GET
    status_code: 200
    user: "{{ db.credentials.admin.user }}"
    password: "{{ db.credentials.admin.pass }}"
    force_basic_auth: yes
  register: cluster_state
  run_once: true

- include_tasks: cluster_setup.yml
  when: cluster_state.json.state == "cluster_enabled"

@rabbah
Copy link
Member Author

rabbah commented Feb 10, 2019

Are the two (this Pr for mounting a file and cluster setup) orthogonal? If so do you want to send a PR for the fix you mention above?

@style95
Copy link
Member

style95 commented Feb 10, 2019

@rabbah
Yes it’s orthogonal.
Ok I will open a new PR.
Actually the above way won’t work when couchdb is scaled out.
Would it be enough for us?

Anyway I will work on it.

@rabbah
Copy link
Member Author

rabbah commented Feb 10, 2019

Ok then I will merge this PR then @style95.

@style95
Copy link
Member

style95 commented Feb 11, 2019

Ok, I will work on it from the merged version.
One thing to note is if this PR is merged, and someone configures persistent mounting for CouchDB, he cannot redeploy CouchDB without manual intervention in a distributed environment.

@rabbah
Copy link
Member Author

rabbah commented Feb 11, 2019

Is there a predicate I can add to the mounting to make it ok now?

For example:
  wskdev couchdb -d -e"db_persist_path=/home/couchd/openwhisk"
@style95
Copy link
Member

style95 commented Feb 12, 2019

@rabbah I think we can add one predicate with the cluster_enabled state as I mentioned above.
This is because it will try to add nodes to the cluster again, we need to skip cluster formation if the cluster is already formed.

Maybe we can register the state and check while proceeding the clustering steps.

- name: enable the cluster setup mode
- name: add remote nodes to the cluster
...
when cluster_state.json.state != "cluster_finished"

This is a simple solution, but it's not possible to add more nodes one the cluster is formed.

How about simply adding above predicate to prevent error?
I will open a PR based on that to enable scale-out as well.

@rabbah
Copy link
Member Author

rabbah commented Feb 12, 2019

As discussed with @style95 on Slack - we will merge this PR as is.

@rabbah rabbah merged commit bebb772 into apache:master Feb 12, 2019
@rabbah rabbah deleted the couchdb branch February 17, 2019 16:15
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
For example:
  wskdev couchdb -d -e"db_persist_path=/home/couchd/openwhisk"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
couchdb review Review for this PR has been requested and yet needs to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create '_users' database for singleton mode:"The database could not be created, the file already exists.
4 participants