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.

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

fix: slow log to info #2935

Closed
wants to merge 13 commits into from
Closed

fix: slow log to info #2935

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c5cbb1f
slow log to info
Oct 28, 2024
d618282
fix ci
Oct 29, 2024
b1163bd
fix ci
Oct 29, 2024
3fcc14e
fix ci
Oct 29, 2024
a4e81f1
fix ci
Oct 29, 2024
60efd65
fix ci
Oct 29, 2024
4522363
fix ci
Oct 29, 2024
c3936d8
fix ci
Oct 29, 2024
b34af68
fix ci
Oct 29, 2024
f5ab80a
fix ci
Oct 29, 2024
1f7caae
fix ci
Oct 29, 2024
caed42d
fix ci
Oct 29, 2024
4d7bf70
fix ci
Oct 29, 2024
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
28 changes: 15 additions & 13 deletions .github/workflows/pika.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ jobs:
sh integrate_test.sh

build_on_macos:
runs-on: macos-12
runs-on: macos-13

steps:
- uses: actions/checkout@v4
Expand All @@ -169,7 +169,7 @@ jobs:
- name: ccache
uses: hendrikmuhs/[email protected]
with:
key: macos-12
key: macos-13

- name: Install Deps
run: |
Expand All @@ -188,7 +188,10 @@ jobs:

- name: Cleanup
run: |
rm -rf ./deps
cp deps/lib/libz.1.dylib .
cp deps/lib/libz.1.dylib tests/integration/
Comment on lines +191 to +192
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Warning: The deps directory copy operations will fail

The workflow shows that the deps directory is cleaned up in a previous step before these copy operations:

      - name: Cleanup
        run: |
          rm -rf ./deps
          rm -rf ./buildtrees

Then immediately after, it attempts to copy files from the already deleted deps/lib/libz.1.dylib. This will always fail as the source files no longer exist.

  • The copy operations in the macOS job need to be moved before the cleanup step that removes the deps directory
  • Alternatively, if the library files are needed for tests, the cleanup step should be moved after the tests are completed
🔗 Analysis chain

Verify dynamic library copy operations.

The copy operations for libz.1.dylib are crucial for the tests. Let's verify the file locations and permissions.

Let me check the workflow file to understand the context of these copy operations better.

Let me check if the deps directory is created in a previous step of the workflow.

Let me check the entire workflow file to understand the complete setup process.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify library files and permissions
echo "Checking source location..."
test -f deps/lib/libz.1.dylib && \
  echo "Source exists: $(ls -l deps/lib/libz.1.dylib)" || \
  echo "Source missing"

echo -e "\nChecking destination locations..."
for path in "./libz.1.dylib" "tests/integration/libz.1.dylib"; do
  test -f "$path" && \
    echo "Destination exists: $(ls -l $path)" || \
    echo "Destination missing: $path"
done

Length of output: 606

Script:

#!/bin/bash
# Check the workflow file content around the copy operations
rg -A 5 -B 5 "libz.1.dylib" .github/workflows/pika.yml

Length of output: 378

Script:

#!/bin/bash
# Check the workflow file for deps directory creation/setup
rg -B 10 "deps/lib" .github/workflows/pika.yml

Length of output: 591

Script:

#!/bin/bash
# Get the complete workflow file content
cat .github/workflows/pika.yml

Length of output: 8181

cp output/pika tests/integration/
rm -rf ./deps
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Warning: Copy operations in cleanup step may be failing

The verification reveals that neither the dynamic library (libz.1.dylib) nor the pika binary are present in the expected locations after the copy operations. This suggests that:

  • The source files in deps/lib/libz.1.dylib and output/pika may not exist when the copy commands are executed
  • The copy operations might be failing silently

Locations checked and found missing:

  • ./libz.1.dylib
  • ./pika
  • tests/integration/libz.1.dylib
  • tests/integration/pika
  • output/pika (directory not found)
🔗 Analysis chain

LGTM: Proper setup of dynamic library and binary dependencies.

The cleanup step now correctly preserves required dependencies before removing the deps directory. Consider verifying the library dependencies of the pika binary.

Let me try to verify the binary and library existence first before making conclusions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify dynamic library dependencies
echo "Checking pika binary dependencies..."
otool -L tests/integration/pika 2>/dev/null || echo "Binary not found or not a Mach-O file"

Length of output: 201

Script:

#!/bin/bash
# Check if files exist and their types
echo "Checking files in root directory:"
file ./libz.1.dylib 2>/dev/null || echo "libz.1.dylib not found in root"
file ./pika 2>/dev/null || echo "pika not found in root"

echo -e "\nChecking files in integration tests directory:"
file tests/integration/libz.1.dylib 2>/dev/null || echo "libz.1.dylib not found in integration tests"
file tests/integration/pika 2>/dev/null || echo "pika not found in integration tests"

echo -e "\nListing output directory:"
ls -la output/ 2>/dev/null || echo "output directory not found"

Length of output: 740

rm -rf ./buildtrees

- name: Test
Expand All @@ -201,19 +204,18 @@ jobs:
./pikatests.sh all clean

- name: Start codis, pika master and pika slave
working-directory: ${{ github.workspace }}/build
run: |
chmod +x ../tests/integration/start_master_and_slave.sh
../tests/integration/start_master_and_slave.sh
chmod +x ../tests/integration/start_codis.sh
../tests/integration/start_codis.sh
working-directory: ${{ github.workspace }}
run: |
cd tests/integration/
chmod +x start_master_and_slave.sh
./start_master_and_slave.sh
chmod +x start_codis.sh
./start_codis.sh

- name: Run Go E2E Tests
working-directory: ${{ github.workspace }}/build
working-directory: ${{ github.workspace }}
run: |
cd ../tools/pika_keys_analysis/
go test -v ./...
cd ../../tests/integration/
cd tests/integration/
chmod +x integrate_test.sh
sh integrate_test.sh

Expand Down
2 changes: 1 addition & 1 deletion src/pika_client_conn.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ void PikaClientConn::ProcessSlowlog(const PikaCmdArgsType& argv, uint64_t do_dur
}
}
}
LOG(ERROR) << "ip_port: " << ip_port() << ", db: " << current_db_ << ", command:" << slow_log
LOG(INFO) << "ip_port: " << ip_port() << ", db: " << current_db_ << ", command:" << slow_log
<< ", command_size: " << cmd_size - 1 << ", arguments: " << argv.size()
<< ", total_time(ms): " << time_stat_->total_time() / 1000
<< ", before_queue_time(ms): " << time_stat_->before_queue_time() / 1000
Expand Down
Loading