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

ADBDEV-4349: gplogfilter: fix timerange validation #1134

Open
wants to merge 1 commit into
base: adb-7.2.0
Choose a base branch
from

Conversation

silent-observer
Copy link

@silent-observer silent-observer commented Nov 27, 2024

gplogfilter: fix timerange validation

Previously the time range filtering of log files was incorrect: there was a check for belonging a log file to specified time range which worked like a binary AND (if time < begin AND time > end - skip file), but something like binary OR is needed here. Thus file filtering never worked. Also this validation was executed after the file was opened, so in case of failed validation there was a redundant open/close operations. This patch fixes such behavior, by:

  • moving the logic of belonging log file name the user specified range to the separate function (it's also helps to cover corner cases)
  • skip file processing, if it doesn't belong to range, at the begin of loop to prevent redundant open/close operations

Changes from original commit:

  1. Fix print() syntax for Python 3
  2. Add StopIteration handler (required by Python 3.7)
  3. Update behave test for GPDB 7 (use COORDINATOR_DATA_DIRECTORY and /log)

(cherry picked from commit e971286)


Note: do not squash to preserve authorship

@RekGRpth
Copy link
Member

ADBDEV-4349: gplogfilter: fix timerange validation (#627)

maybe change to

gplogfilter: fix timerange validation (#1134)

and add this title to PR description?

@silent-observer
Copy link
Author

That's the default way cherry-picked commits work (preserving the PR link from the old PR). Should it always be changed to the new PR?

@RekGRpth
Copy link
Member

That's the default way cherry-picked commits work (preserving the PR link from the old PR). Should it always be changed to the new PR?

yes, to avoid confusion

Previously the time range filtering of log files was incorrect: there
was a check for belonging a log file to specified time range which
worked like a binary `AND` (if time < begin AND time > end - skip
file), but something like binary `OR` is needed here. Thus file
filtering never worked. Also this validation was executed after the
file was opened, so in case of failed validation there was a redundant
`open/close` operations. This patch fixes such behavior, by:
- moving the logic of belonging log file name the user specified range
  to the separate function (it's also helps to cover corner cases)
- skip file processing, if it doesn't belong to range, at the
  begin of loop to prevent redundant `open/close` operations

Changes from original commit:
1. Fix print() syntax for Python 3
2. Add StopIteration handler (required by Python 3.7)
3. Update behave test for GPDB 7 (use COORDINATOR_DATA_DIRECTORY and /log)

(cherry picked from commit e971286)
@silent-observer silent-observer changed the title ADBDEV-4349: gplogfilter: fix timerange validation (#627) ADBDEV-4349: gplogfilter: fix timerange validation Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants