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

about REDIS_RDB_POLICY #72323

Open
allegrox opened this issue Sep 10, 2024 · 3 comments · May be fixed by #72806
Open

about REDIS_RDB_POLICY #72323

allegrox opened this issue Sep 10, 2024 · 3 comments · May be fixed by #72806
Assignees
Labels
in-progress redis tech-issues The user has a technical issue about an application

Comments

@allegrox
Copy link

allegrox commented Sep 10, 2024

Name and Version

bitnami/redis

What architecture are you using?

amd64

What steps will reproduce the bug?

https://github.com/bitnami/containers/blob/main/bitnami/redis/README.md

The readme documentation describes setting Enable Redis(R) RDB persistence in docker-compose.yml: redis.

  ...
    environment: ...
      ...
      - REDIS_TLS_ENABLED=yes
      - REDIS_RDB_POLICY_DISABLED=no
      - REDIS_RDB_POLICY="900#1 600#5 300#10 120#50 60#1000 30#10000”
    ...
  ...

But after installing this method configuration the container fails to start with an error:

redis-1 | redis 17:34:42.20 INFO ==> ** Starting Redis **
redis-1 | redis 17:34:42:20 INFO ==> ** Starting Redis **
redis-1 | *** FATAL CONFIG FILE ERROR (Redis 6.2.14) ***
redis-1 | Reading the configuration file, at line 2055
redis-1 | >>> 'save '900 1'
redis-1 | Unbalanced quotes in configuration line
redis-1 exited with code 1

Changed to the following method to start normally
- “REDIS_RDB_POLICY=900#1 600#5 300#10 120#50 60#1000 30#10000”
But I found another problem, every time I restarted the container, the contents of the CONFIG GET SAVE fetch would be incremented with the contents of REDIS_RDB_POLICY, e.g.:

127.0.0.1:6379> config get save
1) "save"
2) "900 1 600 5 300 10 120 50 60 1000 30 10000 900 1 600 5 300 10 120 50 60 1000 30 10000"

How do I configure the REDIS_RDB_POLICY parameter correctly?

@allegrox allegrox added the tech-issues The user has a technical issue about an application label Sep 10, 2024
@github-actions github-actions bot added the triage Triage is needed label Sep 10, 2024
@github-actions github-actions bot removed the triage Triage is needed label Sep 11, 2024
@github-actions github-actions bot assigned andresbono and unassigned carrodher Sep 11, 2024
@andresbono
Copy link
Contributor

Hi @allegrox, thank you for opening this issue. You are right, setting the "save" config is buggy right now. It can be set multiple times and the resulting applied configuration would be the concatenation of all of those.

To fix the issue, we should probably make the redis_conf_set a bit smarter and detect if the config param is already set in the config file. Also, I'm not really sure why the for loop is in place. Some refs:

#The value stored in $i here is the number of seconds and times of save rules in redis rdb mode
if is_empty_value "$REDIS_RDB_POLICY"; then
if is_boolean_yes "$REDIS_RDB_POLICY_DISABLED"; then
redis_conf_set save ""
fi
else
for i in ${REDIS_RDB_POLICY}; do
redis_conf_set save "${i//#/ }"
done
fi

redis_conf_set() {
local -r key="${1:?missing key}"
local value="${2:-}"
# Sanitize inputs
value="${value//\\/\\\\}"
value="${value//&/\\&}"
value="${value//\?/\\?}"
value="${value//[$'\t\n\r']}"
[[ "$value" = "" ]] && value="\"$value\""
# Determine whether to enable the configuration for RDB persistence, if yes, do not enable the replacement operation
if [ "${key}" == "save" ]; then
echo "${key} ${value}" >> "${REDIS_BASE_DIR}/etc/redis.conf"
else
replace_in_file "${REDIS_BASE_DIR}/etc/redis.conf" "^#*\s*${key} .*" "${key} ${value}" false
fi
}

Would you like to contribute with a fix for this bug? PRs are more than welcome!!

@allegrox
Copy link
Author

In my understanding, $REDIS_RDB_POLICY should contain a full policy content. Then saving the policy content is a process of replacing the existing policy, so you can clear the current policy before starting.
My solution is to copy the script from line 419 to just before line 422.
I'll submit a pull request later.

allegrox added a commit to allegrox/containers that referenced this issue Sep 28, 2024
@carrodher
Copy link
Member

Thank you for opening this issue and submitting the associated Pull Request. Our team will review and provide feedback. Once the PR is merged, the issue will automatically close.

Your contribution is greatly appreciated!

@carrodher carrodher assigned juan131 and unassigned andresbono Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress redis tech-issues The user has a technical issue about an application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants