Skip to content

Remove quicklz related codes. #1188

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zhangwenchao-123
Copy link
Contributor

We decide not to support quicklz compress type anymore.

Authored-by: Zhang Wenchao zhangwenchao@apache.org

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@edespino
Copy link
Contributor

Comparison with Greenplum's QuickLZ Removal Approach

I reviewed how Greenplum handled QuickLZ removal and noticed they took a different, more gradual approach that might be worth considering:

Greenplum's Two-Phase Approach

  1. First commit: Added gp_quicklz_fallback
    GUC for backward compatibility

    • Existing quicklz tables continue working
    • New quicklz specifications automatically convert to zstd
    • Provides graceful degradation path
  2. Second commit: Removed quicklz from build
    system

    • Eliminated build-time support while keeping runtime fallback

This PR's Approach

  • Complete removal without fallback mechanism
  • Cleaner codebase but higher risk of breaking existing installations

Considerations

Pros of current approach:

  • Cleaner code, no technical debt
  • Clear break from unsupported feature

Potential concerns:

  • Existing tables with quicklz compression may fail on upgrade
  • No migration path for users
  • Could cause immediate failures without clear guidance

Suggestions

  1. If keeping current approach: Add clear error messages pointing to migration docs
    ERROR: QuickLZ compression is no longer supported.
    Please use 'zstd' or 'zlib' compression instead.
    For migration guidance, see: [documentation link]
  2. Alternative: Consider implementing a temporary fallback mechanism similar to Greenplum's approach for smoother user transition

What's the team's preference on backward compatibility vs. clean removal for this change?

@tuhaihe
Copy link
Member

tuhaihe commented Jun 25, 2025

We decide not to support quicklz compress type anymore.

Hi @zhangwenchao-123, I'm confused about the word we. Is there a public thread tracking the decision process?

@zhangwenchao-123
Copy link
Contributor Author

We decide not to support quicklz compress type anymore.

Hi @zhangwenchao-123, I'm confused about the word we. Is there a public thread tracking the decision process?

As quicklz has some license argument, it's better to disable it.

@tuhaihe
Copy link
Member

tuhaihe commented Jun 25, 2025

We decide not to support quicklz compress type anymore.

Hi @zhangwenchao-123, I'm confused about the word we. Is there a public thread tracking the decision process?

As quicklz has some license argument, it's better to disable it.

That makes sense. It is important to revise your commit message body to provide sufficient context.

@zhangwenchao-123
Copy link
Contributor Author

Comparison with Greenplum's QuickLZ Removal Approach

I reviewed how Greenplum handled QuickLZ removal and noticed they took a different, more gradual approach that might be worth considering:

Greenplum's Two-Phase Approach

  1. First commit: Added gp_quicklz_fallback
    GUC for backward compatibility

    • Existing quicklz tables continue working
    • New quicklz specifications automatically convert to zstd
    • Provides graceful degradation path
  2. Second commit: Removed quicklz from build
    system

    • Eliminated build-time support while keeping runtime fallback

This PR's Approach

  • Complete removal without fallback mechanism
  • Cleaner codebase but higher risk of breaking existing installations

Considerations

Pros of current approach:

  • Cleaner code, no technical debt
  • Clear break from unsupported feature

Potential concerns:

  • Existing tables with quicklz compression may fail on upgrade
  • No migration path for users
  • Could cause immediate failures without clear guidance

Suggestions

  1. If keeping current approach: Add clear error messages pointing to migration docs
    ERROR: QuickLZ compression is no longer supported.
    Please use 'zstd' or 'zlib' compression instead.
    For migration guidance, see: [documentation link]
  2. Alternative: Consider implementing a temporary fallback mechanism similar to Greenplum's approach for smoother user transition

What's the team's preference on backward compatibility vs. clean removal for this change?

The reason why we do this is as the quicklz has license argument, so it's better to remove all of the codes which have quicklz key words. As we have changed some files such as CI related files in our repo, it can't cherry-pick from gp upstream directly, this approach may be better. For migration, we need time to discuss how to achieve it, but maybe we should remove quicklz firstly to avoid the danger of unnecessary license argument.

@jianlirong
Copy link

My personal view leans towards completely removing QuickLZ from both the code and product level in the upcoming 2.0 release, rather than deliberately adopting the gentle, gradual elimination approach similar to GPDB. There are several main reasons for this:

  1. Cloudberry already supports the ZSTD compression algorithm, which can match QuickLZ in both decompression performance and compression ratio.
  2. Whether migrating from GPDB or Cloudberry 1.6 to Cloudberry 2.0, we don't provide in-place upgrade functionality. In other words, this migration process inevitably involves data migration, and during the data migration process, we can completely use ZSTD to replace QuickLZ in Cloudberry 2.0. The biggest benefit of maintaining compatibility with the old version's QuickLZ no longer exists.
  3. 2.0 is the first official release of Cloudberry after joining the AFS incubator, which is a good opportunity to draw a clear line with QuickLZ. As long as this version doesn't provide QuickLZ compressed tables, users won't raise this issue in the future. Conversely, if this version leaves this gap, once users use QuickLZ compressed tables, subsequent upgrades will always be a problem, especially for users with large amounts of data.

We decide not to support quicklz compress type anymore.

Authored-by: Zhang Wenchao zhangwenchao@apache.org
@tuhaihe tuhaihe self-requested a review July 1, 2025 07:28
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.

4 participants