Skip to content

fix:CI #3199

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.

Sign up for GitHub

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

Closed
chejinge wants to merge 9 commits into from
Closed

fix:CI #3199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e5bab9f
fix:CI
brother-jin Dec 1, 2025
07e43d6
fix:CI
brother-jin Dec 1, 2025
fa09ec8
fixci
brother-jin Dec 1, 2025
192207f
fixci
brother-jin Dec 1, 2025
592eb8d
fixci
brother-jin Dec 1, 2025
c245ae7
fixci
brother-jin Dec 1, 2025
8d24ea0
fixci
brother-jin Dec 2, 2025
fb85736
fixci
brother-jin Dec 2, 2025
7a18d22
fix
brother-jin Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions .github/workflows/pika.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ jobs:
# Build your program with the given configuration
run: cmake --build build --config ${{ env.BUILD_TYPE }}

- name: Cleanup
- name: Cleanup Build Artifacts
run: |
rm -rf ./buildtrees
rm -rf ./deps/src
rm -rf ./build/CMakeFiles
df -h

- uses: actions/upload-artifact@v4
with:
Expand All @@ -65,6 +68,17 @@ jobs:
run: ./pikatests.sh all clean

# master on port 9221, slave on port 9231, all with 2 db
- name: Check Disk Space
run: |
echo "Checking disk space before starting codis..."
df -h
echo "Cleaning up additional space if needed..."
docker system prune -af || true
sudo rm -rf /usr/share/dotnet || true
sudo rm -rf /opt/ghc || true
sudo rm -rf /usr/local/share/boost || true
df -h

- name: Start codis, pika master and pika slave
working-directory: ${{ github.workspace }}/build
run: |
Expand Down Expand Up @@ -105,9 +119,12 @@ jobs:
source /opt/rh/devtoolset-10/enable
cmake --build build --config ${{ env.BUILD_TYPE }}

- name: Cleanup
- name: Cleanup Build Artifacts
run: |
rm -rf ./buildtrees
rm -rf ./deps/src
rm -rf ./build/CMakeFiles
df -h

- name: Test
working-directory: ${{ github.workspace }}/build
Expand All @@ -117,6 +134,14 @@ jobs:
working-directory: ${{ github.workspace }}
run: ./pikatests.sh all clean

- name: Check Disk Space
run: |
echo "Checking disk space before starting codis..."
df -h
echo "Cleaning up additional space if needed..."
rm -rf /tmp/* || true
df -h

- name: Start codis, pika master and pika slave
working-directory: ${{ github.workspace }}/build
run: |
Expand Down Expand Up @@ -165,11 +190,14 @@ jobs:
run: |
cmake --build build --config ${{ env.BUILD_TYPE }}

- name: Cleanup
- name: Cleanup Build Artifacts
run: |
cp deps/lib/libz.1.dylib .
cp deps/lib/libz.1.dylib tests/integration/
rm -rf ./buildtrees
rm -rf ./deps/src
rm -rf ./build/CMakeFiles
df -h

- name: Test
working-directory: ${{ github.workspace }}/build
Expand Down
6 changes: 5 additions & 1 deletion pikatests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ function cleanup() {
rm -rf ./log[0-9]*
rm -rf ./db[0-9]*
rm -rf dbsync/
rm src/redis-server
rm -f src/redis-server
echo "Cleanup completed"
}

# Add trap to ensure cleanup is executed on script exit, even if there's an error
trap cleanup EXIT

# check if tcl is installed
function check_tcl {
if [ -z "$(which tclsh)" ]; then
Expand Down
16 changes: 8 additions & 8 deletions tests/conf/pika.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ db-path : ./db/
# but this will generate heavier IO load when flushing from buffer to disk,
# you should configure it based on you usage scenario.
# Supported Units [K|M|G], write-buffer-size default unit is in [bytes].
write-buffer-size : 256M
write-buffer-size : 64M

# The size of one block in arena memory allocation.
# If <= 0, a proper value is automatically calculated.
Expand Down Expand Up @@ -150,7 +150,7 @@ maxclients : 20000
# the price is that the number of sst files could be huge. On the contrary, the bigger the sst file size, the lower
# the performance and the higher the merge cost, while the number of files is fewer.
# Supported Units [K|M|G], target-file-size-base default unit is in [bytes] and the default value is 20M.
target-file-size-base : 20M
target-file-size-base : 10M

# Expire-time of binlog(write2file) files that stored within log-path.
# Any binlog(write2file) files that exceed this expire time will be cleaned up.
Expand All @@ -163,7 +163,7 @@ expire-logs-days : 7
# automatic cleaning will start to ensure the maximum number
# of binlog files is equal to expire-logs-nums.
# The [Minimum value] of this parameter is 10.
expire-logs-nums : 10
expire-logs-nums : 3

# The number of guaranteed connections for root user.
# This parameter guarantees that there are 2(By default) connections available
Expand Down Expand Up @@ -280,7 +280,7 @@ write-binlog : yes
# [NOTICE] Master and slaves must have exactly the same value for the binlog-file-size.
# The [value range] of binlog-file-size is [1K, 2G].
# Supported Units [K|M|G], binlog-file-size default unit is in [bytes] and the default value is 100M.
binlog-file-size : 104857600
binlog-file-size : 10485760

# Automatically triggers a small compaction according to statistics
# Use the cache to store up to 'max-cache-statistic-keys' keys
Expand All @@ -299,7 +299,7 @@ small-compaction-duration-threshold : 10000
# exceeds max-write-buffer-size when next write operation is issued.
# [RocksDB-Basic-Tuning](https://github.com/facebook/rocksdb/wiki/Setup-Options-and-Basic-Tuning)
# Supported Units [K|M|G], max-write-buffer-size default unit is in [bytes].
max-write-buffer-size : 10737418240
max-write-buffer-size : 1073741824

# The maximum number of write buffers(memtables) that are built up in memory for one ColumnFamily in DB.
# The default and the minimum number is 2. It means that Pika(RocksDB) will write to a write buffer
Expand Down Expand Up @@ -365,7 +365,7 @@ max-background-compactions : 2
max-background-jobs : 3

# maximum value of RocksDB cached open file descriptors
max-cache-files : 5000
max-cache-files : 1000

# The ratio between the total size of RocksDB level-(L+1) files and the total size of RocksDB level-L files for all L.
# Its default value is 10(x). You can also change it to 5(x).
Expand Down Expand Up @@ -520,8 +520,8 @@ zset-cache-field-num-per-key : 512
# If zset-cache-start-direction is -1, cache the last 512[zset-cache-field-num-per-key] elements
zset-cache-start-direction : 0

# the cache maxmemory of every db, configuration 10G
cache-maxmemory : 10737418240
# the cache maxmemory of every db, configuration 512M
cache-maxmemory : 536870912

# cache-maxmemory-policy
# 0: volatile-lru -> Evict using approximated LRU among the keys with an expire set.
Expand Down
66 changes: 55 additions & 11 deletions tests/integration/start_codis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
#!/bin/bash

#pkill -9 pika
#pkill -9 codis
#rm -rf /tmp/codis
#rm -rf codis_data_1
#rm -rf codis_data_2
# Clean up processes and temporary files
pkill -9 pika || true
pkill -9 codis || true
# Wait for processes to fully terminate and release resources
sleep 5
rm -rf /tmp/codis || true
rm -rf codis_data_1 || true
rm -rf codis_data_2 || true

# Clean up log directory in codis to free space, but keep bin directory
# to preserve existing codis binaries
rm -rf ../codis/log || true
mkdir -p ../codis/bin
mkdir -p ../codis/log

# Display memory usage for monitoring
free -h || true

# Display available disk space
df -h

CODIS_DASHBOARD_ADDR=127.0.0.1:18080

Expand All @@ -21,15 +36,47 @@ mkdir codis_data_1
mkdir codis_data_2

# Example Change the location for storing data on primary and secondary nodes in the configuration file
Comment thread
coderabbitai[bot] marked this conversation as resolved.
sed -i '' -e 's|databases : 1|databases : 2|' -e 's|port : 9221|port : 8000|' -e 's|log-path : ./log/|log-path : ./codis_data_1/log/|' -e 's|db-path : ./db/|db-path : ./codis_data_1/db/|' -e 's|dump-path : ./dump/|dump-path : ./codis_data_1/dump/|' -e 's|pidfile : ./pika.pid|pidfile : ./codis_data_1/pika.pid|' -e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_1/dbsync/|' -e 's|#daemonize : yes|daemonize : yes|' ./pika_8000.conf
sed -i '' -e 's|databases : 1|databases : 2|' -e 's|port : 9221|port : 8001|' -e 's|log-path : ./log/|log-path : ./codis_data_2/log/|' -e 's|db-path : ./db/|db-path : ./codis_data_2/db/|' -e 's|dump-path : ./dump/|dump-path : ./codis_data_2/dump/|' -e 's|pidfile : ./pika.pid|pidfile : ./codis_data_2/pika.pid|' -e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_2/dbsync/|' -e 's|#daemonize : yes|daemonize : yes|' ./pika_8001.conf
# Start three nodes
# Reduce memory usage for CI environment
sed -i.bak \
-e 's|databases : 1|databases : 2|' \
-e 's|port : 9221|port : 8000|' \
-e 's|log-path : ./log/|log-path : ./codis_data_1/log/|' \
-e 's|db-path : ./db/|db-path : ./codis_data_1/db/|' \
-e 's|dump-path : ./dump/|dump-path : ./codis_data_1/dump/|' \
-e 's|pidfile : ./pika.pid|pidfile : ./codis_data_1/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_1/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' \
./pika_8000.conf
Comment on lines +40 to +52
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Dec 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clean up sed backup files to prevent CI disk waste.

Both sed commands use -i.bak, creating backup files (.bak) that persist on disk. This was previously flagged as a major resource-cleanup concern and contradicts the PR goal to tighten CI resource usage. Remove the backup files after each sed invocation:

 sed -i.bak \
   -e 's|databases : 1|databases : 2|' \
   -e 's|port : 9221|port : 8000|' \
   -e 's|log-path : ./log/|log-path : ./codis_data_1/log/|' \
   -e 's|db-path : ./db/|db-path : ./codis_data_1/db/|' \
   -e 's|dump-path : ./dump/|dump-path : ./codis_data_1/dump/|' \
   -e 's|pidfile : ./pika.pid|pidfile : ./codis_data_1/pika.pid|' \
   -e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_1/dbsync/|' \
   -e 's|#daemonize : yes|daemonize : yes|' \
   -e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
   -e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
   -e 's|write-buffer-size : 256M|write-buffer-size : 64M|' \
   ./pika_8000.conf
+rm -f ./pika_8000.conf.bak
 
 sed -i.bak \
   -e 's|databases : 1|databases : 2|' \
   -e 's|port : 9221|port : 8001|' \
   -e 's|log-path : ./log/|log-path : ./codis_data_2/log/|' \
   -e 's|db-path : ./db/|db-path : ./codis_data_2/db/|' \
   -e 's|dump-path : ./dump/|dump-path : ./codis_data_2/dump/|' \
   -e 's|pidfile : ./pika.pid|pidfile : ./codis_data_2/pika.pid|' \
   -e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_2/dbsync/|' \
   -e 's|#daemonize : yes|daemonize : yes|' \
   -e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
   -e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
   -e 's|write-buffer-size : 256M|write-buffer-size : 64M|' \
   ./pika_8001.conf
+rm -f ./pika_8001.conf.bak

Alternatively, if the original config files do not need to be preserved, use sed -i without the backup suffix.

Also applies to: 54-66

🤖 Prompt for AI Agents 
In tests/integration/start_codis.sh around lines 40 to 52 (and similarly for
lines 54 to 66), the sed invocations use -i.bak which leaves .bak backup files
on disk; change the sed calls to either use in-place editing without a backup
(sed -i) if preserving originals is unnecessary, or keep -i.bak but delete the
generated .bak files immediately after each sed command (e.g., rm -f
./pika_8000.conf.bak and other .bak files), ensuring no leftover .bak files
remain in CI to save disk space.


sed -i.bak \
-e 's|databases : 1|databases : 2|' \
-e 's|port : 9221|port : 8001|' \
-e 's|log-path : ./log/|log-path : ./codis_data_2/log/|' \
-e 's|db-path : ./db/|db-path : ./codis_data_2/db/|' \
-e 's|dump-path : ./dump/|dump-path : ./codis_data_2/dump/|' \
-e 's|pidfile : ./pika.pid|pidfile : ./codis_data_2/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./codis_data_2/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' \
./pika_8001.conf

# Start instances one by one with time to initialize
./pika -c ./pika_8000.conf
sleep 5
./pika -c ./pika_8001.conf
#ensure both master and slave are ready
sleep 10

# Display memory usage after starting pika instances
free -h || true

cd ../codis
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Dec 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add error handling to cd command.

If the directory change fails, subsequent commands will execute in the wrong location, causing silent failures. Add error handling to exit or return on failure.

-cd ../codis
+cd ../codis || exit 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cd ../codis
cd ../codis || exit 1
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 56-56: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

🤖 Prompt for AI Agents 
In tests/integration/start_codis.sh around line 56, the script runs "cd
../codis" without checking for failure; update the script to check the exit
status of the cd command and abort on failure (e.g., test the return value or
use "cd ../codis || { echo 'Failed to change directory to ../codis' >&2; exit 1;
}") so subsequent commands do not run in the wrong directory.

echo "Building codis..."
make

echo 'startup codis dashboard and codis proxy'
Expand All @@ -55,6 +102,3 @@ echo 'resync all groups'

#ensure codis are ready
sleep 10



35 changes: 28 additions & 7 deletions tests/integration/start_master_and_slave.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ mkdir slave_data
sed -i.bak \
-e 's|databases : 1|databases : 2|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_single.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_single.conf

sed -i.bak \
-e 's|databases : 1|databases : 2|' \
Expand All @@ -26,7 +29,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./master_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./master_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_master.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_master.conf

sed -i.bak \
-e 's|databases : 1|databases : 2|' \
Expand All @@ -37,7 +43,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./slave_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./slave_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_slave.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_slave.conf

sed -i.bak \
-e 's|# rename-command : FLUSHALL 360flushall|rename-command : FLUSHALL 360flushall|' \
Expand All @@ -50,7 +59,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./rename_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./rename_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_rename.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_rename.conf

sed -i.bak \
-e 's|requirepass :|requirepass : requirepass|' \
Expand All @@ -64,7 +76,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./acl1_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./acl1_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_acl_both_password.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_acl_both_password.conf

sed -i.bak \
-e 's|requirepass :|requirepass : requirepass|' \
Expand All @@ -77,7 +92,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./acl2_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./acl2_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_acl_only_admin_password.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_acl_only_admin_password.conf
sed -i.bak \
-e 's|requirepass :|requirepass : requirepass|' \
-e 's|masterauth :|masterauth : requirepass|' \
Expand All @@ -90,7 +108,10 @@ sed -i.bak \
-e 's|pidfile : ./pika.pid|pidfile : ./acl3_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./acl3_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' \
-e 's|timeout : 60|timeout : 500|' ./pika_has_other_acl_user.conf
-e 's|timeout : 60|timeout : 500|' \
-e 's|cache-maxmemory : 10737418240|cache-maxmemory : 268435456|' \
-e 's|max-write-buffer-size : 10737418240|max-write-buffer-size : 268435456|' \
-e 's|write-buffer-size : 256M|write-buffer-size : 64M|' ./pika_has_other_acl_user.conf
echo -e '\nuser : limit on >limitpass ~* +@all &*' >> ./pika_has_other_acl_user.conf

# Start three nodes
Expand Down
Loading