Skip to content
Closed

fix:CI #3199

Show file tree
Hide file tree
Changes from 6 commits
Commits
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
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
11 changes: 10 additions & 1 deletion codis/admin/codis-dashboard-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,18 @@ CODIS_DASHBOARD_CONF_FILE=$CODIS_CONF_DIR/dashboard.toml
echo $CODIS_DASHBOARD_CONF_FILE

if [ ! -d $CODIS_LOG_DIR ]; then
mkdir -p $CODIS_LOG_DIR
# Try to create directory, but handle failure gracefully
mkdir -p $CODIS_LOG_DIR || {
echo "WARNING: Failed to create log directory at $CODIS_LOG_DIR"
# Use /tmp directory as fallback
CODIS_LOG_DIR="/tmp"
echo "Using fallback directory: $CODIS_LOG_DIR"
}
fi

# Clean up old log files to save space
find $CODIS_LOG_DIR -name "codis-*.log" -type f -mtime +1 -delete 2>/dev/null || true


case $1 in
start)
Expand Down
11 changes: 10 additions & 1 deletion codis/admin/codis-fe-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@ CODIS_FE_ADDR="0.0.0.0:9090"
echo $CODIS_FE_CONF_FILE

if [ ! -d $CODIS_LOG_DIR ]; then
mkdir -p $CODIS_LOG_DIR
# Try to create directory, but handle failure gracefully
mkdir -p $CODIS_LOG_DIR || {
echo "WARNING: Failed to create log directory at $CODIS_LOG_DIR"
# Use /tmp directory as fallback
CODIS_LOG_DIR="/tmp"
echo "Using fallback directory: $CODIS_LOG_DIR"
}
fi

# Clean up old log files to save space
find $CODIS_LOG_DIR -name "codis-*.log" -type f -mtime +1 -delete 2>/dev/null || true


case $1 in
start)
Expand Down
11 changes: 10 additions & 1 deletion codis/admin/codis-proxy-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,18 @@ CODIS_DASHBOARD_ADDR="127.0.0.1:18080"
echo $CODIS_PROXY_CONF_FILE

if [ ! -d $CODIS_LOG_DIR ]; then
mkdir -p $CODIS_LOG_DIR
# Try to create directory, but handle failure gracefully
mkdir -p $CODIS_LOG_DIR || {
echo "WARNING: Failed to create log directory at $CODIS_LOG_DIR"
# Use /tmp directory as fallback
CODIS_LOG_DIR="/tmp"
echo "Using fallback directory: $CODIS_LOG_DIR"
}
fi

# Clean up old log files to save space
find $CODIS_LOG_DIR -name "codis-*.log" -type f -mtime +1 -delete 2>/dev/null || true


case $1 in
start)
Expand Down
6 changes: 5 additions & 1 deletion pikatests.sh
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
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
77 changes: 65 additions & 12 deletions tests/integration/start_codis.sh
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 temporary directories in codis to free space
rm -rf ../codis/bin || true
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,16 +36,57 @@ 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

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

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.

make
echo "Building codis..."

# Free up memory before building
echo "Clearing caches to free up memory..."
sync
if [ $(id -u) -eq 0 ]; then
echo 3 > /proc/sys/vm/drop_caches || true
fi

# Limit parallel jobs during make to reduce memory usage
make -j2

echo 'startup codis dashboard and codis proxy'
./admin/codis-dashboard-admin.sh start
Expand All @@ -55,6 +111,3 @@ echo 'resync all groups'

#ensure codis are ready
sleep 10



Loading