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

ansible-scylla-node: Use Scylla API for getting existent tokens instead of nodetool ring #308

Merged

Conversation

igorribeiroduarte
Copy link
Collaborator

No description provided.

@igorribeiroduarte igorribeiroduarte force-pushed the improve_tokens_generation branch 2 times, most recently from 30289f5 to 7e590b6 Compare December 6, 2023 17:32
Copy link
Collaborator

@vladzcloudius vladzcloudius left a comment

Choose a reason for hiding this comment

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

Looks good in general. There are some comments though.

copy:
content: ""
dest: /tmp/tokens_file
delegate_to: localhost
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work well when multiple concurrent instances of the job would be running.
Generate a temp file instead, please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

dest: /tmp/tokens_file
delegate_to: localhost

- name: Get existent tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

spelling: s/existent/existing/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

uri:
url: "http://{{ scylla_api_address }}:{{ scylla_api_port }}/storage_service/tokens/{{ item }}"
method: GET
register: _existent_tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

ansible-scylla-node/tasks/generate_tokens.yml Show resolved Hide resolved
@igorribeiroduarte igorribeiroduarte force-pushed the improve_tokens_generation branch 2 times, most recently from 1a4ce9c to f91ad63 Compare December 6, 2023 18:25
…during tokens generation

This patch changes the role to stop validating that all nodes are in UN state during tokens
generation, since the nodes don't have to be up in order for us to be able to retrieve
existing tokens and generate the new ones.
…ad of nodetool ring

Currently we're using nodetool ring to get the current tokens and iterating over
its output in order to create an ansible fact with all the tokens.
This might be a problem when using ansible verbose mode since nodetool ring prints
one token per line and every time we append a value to the ansible fact the whole
fact will be printed resulting in a very big number of lines printed, specially
for big clusters.
This patch fixes this problem by using the API to get the current tokens and by writing
them to a file instead of to an ansible fact.
@igorribeiroduarte igorribeiroduarte force-pushed the improve_tokens_generation branch from f91ad63 to 986eb28 Compare December 6, 2023 18:30
@vladzcloudius vladzcloudius merged commit a31e1f8 into scylladb:master Dec 6, 2023
1 of 2 checks passed
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