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

CLOUD-464: Updates for Reconciliation #176

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

mronstro
Copy link
Collaborator

No description provided.

@mronstro
Copy link
Collaborator Author

The following changes have been made:

  1. The pidfile names are now named after the binary name, thus
    ndb_mgmd.pid, ndbmtd.pid and mysqld.pid. To achieve this the
    ndb_mgmd and ndbmtd uses a new parameter --service-name
    that is set to the binary name. This makes it possible to have multiple
    ndbmtd services on the same machine with different service names.
    The change to the systemd script is that a new parameter SERVICE_ARG
    is added (environment variable). The ndb-agent uses this environment
    variable to set the service name. Older implementations simply doesn't
    set the service name and gets the old behaviour.

For MySQL Server we change the my-ndb.cnf file to use a mysqld.pid
as the pidfile name, a new parameter is added for this. Also new parameters
are added for pidfile name of ndbmtd and ndb_mgmd.

@mronstro
Copy link
Collaborator Author

  1. The ServerPort have changed from 10000 to 11860 which is the standard port number to use for this.
  2. The download tarball have added glibcVersion and cpu_platform to accomodate the change to glibcVersion
    2.28 and the addition of arm64_v8 to the CPU platforms.

@mronstro
Copy link
Collaborator Author

  1. The systemd scripts also now accepts INITIAL_START environment variable to ensure that the restart is an
    initial restart and not a normal restart. The old NDBD_INITIAL_START is still accepted as well and they will
    cause the same behaviour.

default['ndb']['majorVersion'] = "21"
default['ndb']['minorVersion'] = "04"
default['ndb']['patchVersion'] = "15"
default['ndb']['majorVersion'] = "22"
Copy link
Contributor

Choose a reason for hiding this comment

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

version change should wait for RONDB-519, which updates rondb from the entire platform

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

templates/default/my-ndb.cnf.erb Outdated Show resolved Hide resolved
templates/default/mysqld.service.erb Outdated Show resolved Hide resolved
templates/default/ndb_mgmd.service.erb Outdated Show resolved Hide resolved
if [ "${NDBD_INITIAL_RESTART}" = "true" ]; then
INIT_ARG=--initial
sed -i 's/^NDBD_INITIAL_RESTART=.*$/NDBD_INITIAL_RESTART=false/g' <%= node['ndb']['scripts_dir'] %>/ndbd_env_variables
elif [ -n "${INITIAL_START}" ] ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

You can combine this in the same if condition and avoid duplicating code ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think the only place where we use NDBD_INITIAL_RESTART is ec2-init and ndb-agent so I think shortly after everything is merged we can continue with only one variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, made an attempt, but a bit uncertain about the exact syntax to use

templates/default/ndbmtd.service.erb Outdated Show resolved Hide resolved
recipes/mgmd.rb Outdated
@@ -101,7 +101,7 @@
group "root"
mode 0754
cookbook 'ndb'
variables({ :node_id => found_id })
variables({})
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this line entirely ;)

@@ -8,11 +8,17 @@ if [ "X$USERID" != "X<%= node['ndb']['user'] %>" ]; then
exit -3
fi

if [ "${NDBD_INITIAL_RESTART}" = "true" ]; then
INIT_ARG=
if [ "${NDBD_INITIAL_RESTART}" = "true" || -n "${INITIAL_START}" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The correct syntax is either [[ "${NDBD_INITIAL_RESTART}" = "true" || -n "${INITIAL_START}" ]] note the double square brackets or [ "${NDBD_INITIAL_RESTART}" = "true" ] || [ -n "${INITIAL_START}" ]

templates/default/mgm-server-start.sh.erb Outdated Show resolved Hide resolved
templates/default/mgm-server-start.sh.erb Show resolved Hide resolved
templates/default/mgm-server-start.sh.erb Outdated Show resolved Hide resolved
templates/default/ndb_mgmd.service.erb Outdated Show resolved Hide resolved
templates/default/ndbd-init.sh.erb Outdated Show resolved Hide resolved
templates/default/ndbd-start.sh.erb Outdated Show resolved Hide resolved
templates/default/ndbmtd.service.erb Outdated Show resolved Hide resolved
templates/default/mysqld.service.erb Outdated Show resolved Hide resolved
templates/default/ndbd-start.sh.erb Outdated Show resolved Hide resolved
templates/default/mysqld.service.erb Outdated Show resolved Hide resolved
@olapiv
Copy link

olapiv commented Jan 25, 2024

The ndb-agent expects the following files to exist under /srv/hops/mysql-cluster/ndb/scripts/:

mgmd_env_variables

MGM_CONN_STRING=
NDB_MGMD_NODE_ID=

INITIAL_START=
SERVICE_NAME=

mysqld_env_variables

MGM_CONN_STRING=

ndbd_env_variables

MGM_CONN_STRING=
NDB_NDBD_NODE_ID=

INITIAL_START=
SERVICE_NAME=

@olapiv
Copy link

olapiv commented Jan 25, 2024

The reason for the service name is to avoid having to sed pid files everywhere. It makes the ndb-agent difficult to reliably test locally since we use supervisord instead of systemd. supervisord runs processes in foreground, so there's no .pid file.

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.

6 participants