Skip to content

Conversation

kamil-holubicki
Copy link
Contributor

@kamil-holubicki kamil-holubicki commented Mar 14, 2025

K8SPXC-1327 Powered by Pull Request Badge

https://perconadev.atlassian.net/browse/K8SPXC-1327

Problem:
By default, standard libc allocator is used by PXC. It is often desirable to use jemalloc instead. To do so, use has to configure LD_PRELOAD environment variable. The only way to do this is through k8s secret.

Solution:
Extended PodSpec object with MySqlAllocator property. If it is empty, omitted or set to 'jemalloc', LD_PRELOAD env variable pointing to jemalloc library will be exported.
Otherwise the default allocator will be used.

CHANGE DESCRIPTION

Problem:
Short explanation of the problem.

Cause:
Short explanation of the root cause of the issue if applicable.

Solution:
Short explanation of the solution we are providing with this PR.

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?
  • Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
  • Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

  • Is an E2E test/test case added for the new feature/change?
  • Are unit tests added where appropriate?
  • Are OpenShift compare files changed for E2E tests (compare/*-oc.yml)?

Config/Logging/Testability

  • Are all needed new/changed options added to default YAML files?
  • Are all needed new/changed options added to the Helm Chart?
  • Did we add proper logging messages for operator actions?
  • Did we ensure compatibility with the previous version or cluster upgrade process?
  • Does the change support oldest and newest supported PXC version?
  • Does the change support oldest and newest supported Kubernetes version?

https://perconadev.atlassian.net/browse/K8SPXC-1327

Problem:
By default, standard libc allocator is used by PXC. It is often
desirable to use jemalloc instead. To do so, use has to configure
LD_PRELOAD environment variable. The only way to do this is through
k8s secret.

Solution:
Extended PodSpec object with MySqlAllocator property. If it is empty,
omitted or set to 'jemalloc', LD_PRELOAD env variable pointing
to jemalloc library will be exported.
Otherwise the default allocator will be used.
@it-percona-cla
Copy link

it-percona-cla commented Mar 14, 2025

CLA assistant check
All committers have signed the CLA.

@pull-request-size pull-request-size bot added the size/L 100-499 lines label Mar 14, 2025
@kamil-holubicki kamil-holubicki requested review from egegunes and removed request for egegunes March 14, 2025 14:43
deploy/cr.yaml Outdated
size: 3
image: perconalab/percona-xtradb-cluster-operator:main-pxc8.0
autoRecovery: true
# mySqlAllocator: jemalloc
Copy link
Collaborator

Choose a reason for hiding this comment

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

In cr we have a commented option with a default value. In our doc, we will add all possible values. Also, you can add them in CRD. It is possible to add validation on CRD level.

Copy link
Contributor Author

@kamil-holubicki kamil-holubicki Mar 17, 2025

Choose a reason for hiding this comment

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

In cr we have a commented option with a default value. In our doc, we will add all possible values.

Done

Also, you can add them in CRD. It is possible to add validation on CRD level.

I'm not sure I understand where and what to add.

@pull-request-size pull-request-size bot added size/M 30-99 lines and removed size/L 100-499 lines labels Mar 17, 2025
egegunes
egegunes previously approved these changes Mar 28, 2025
@egegunes egegunes modified the milestones: v1.17.0, v1.18.0 Mar 28, 2025
Comment on lines 308 to 313
appc.Env = append(appc.Env, []corev1.EnvVar{
{
Name: LD_PRELOAD_KEY,
Value: ldPreloadValue,
},
}...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are setting only one env var, we can simplify this using the following:

	appc.Env = append(appc.Env, corev1.EnvVar{
		Name:  LD_PRELOAD_KEY,
		Value: finalLDPreload,
	})

Comment on lines 271 to 272
const LD_PRELOAD_KEY = "LD_PRELOAD"
const LIBJEMALLOC_PATH = "/usr/lib64/libjemalloc.so.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

The convention is to use camel case to define constants. They can also start with lowercase given that their scope is within the if clause of 1.18 version.

We can also group them like this:

const (
	ldPreloadKey   = "LD_PRELOAD"
	libJemalloc    = "/usr/lib64/libjemalloc.so.1"
)

Comment on lines 285 to 291
// Env vars are set via secret. Check if LD_PRELOAD is set.
for key, value := range envVarsSecret.Data {
if key == LD_PRELOAD_KEY {
ldPreloadValue = string(value)
break
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can avoid this loop completely by using

if val, ok := envSecret.Data[LdPreloadKey]; ok {
    ldPreload = string(val)
}

Data of secret are of type map[string][]byte .

gkech
gkech previously approved these changes Mar 31, 2025
@JNKPercona
Copy link
Collaborator

Test name Status
affinity-8-0 failure
auto-tuning-8-0 failure
cross-site-8-0 failure
custom-users-8-0 failure
demand-backup-cloud-8-0 failure
demand-backup-encrypted-with-tls-8-0 failure
demand-backup-8-0 failure
demand-backup-flow-control-8-0 failure
demand-backup-parallel-8-0 failure
haproxy-5-7 skipped
haproxy-8-0 skipped
init-deploy-5-7 skipped
init-deploy-8-0 skipped
limits-8-0 skipped
monitoring-2-0-8-0 skipped
one-pod-5-7 skipped
one-pod-8-0 skipped
pitr-8-0 skipped
pitr-gap-errors-8-0 skipped
proxy-protocol-8-0 skipped
proxysql-sidecar-res-limits-8-0 skipped
pvc-resize-5-7 skipped
pvc-resize-8-0 skipped
recreate-8-0 skipped
restore-to-encrypted-cluster-8-0 skipped
scaling-proxysql-8-0 skipped
scaling-8-0 skipped
scheduled-backup-5-7 skipped
scheduled-backup-8-0 skipped
security-context-8-0 skipped
smart-update1-8-0 skipped
smart-update2-8-0 skipped
storage-8-0 skipped
tls-issue-cert-manager-ref-8-0 skipped
tls-issue-cert-manager-8-0 skipped
tls-issue-self-8-0 skipped
upgrade-consistency-8-0 skipped
upgrade-haproxy-5-7 skipped
upgrade-haproxy-8-0 skipped
upgrade-proxysql-5-7 skipped
upgrade-proxysql-8-0 skipped
users-5-7 skipped
users-8-0 skipped
validation-hook-8-0 skipped
We run 9 out of 44

commit: 9ccc2eb
image: perconalab/percona-xtradb-cluster-operator:PR-2002-9ccc2eb7

@hors hors modified the milestones: v1.18.0, v1.19.0 Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M 30-99 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants