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

meta: fix empty value in xattr #5696

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

meta: fix empty value in xattr #5696

wants to merge 4 commits into from

Conversation

jiefenghuang
Copy link
Contributor

close #5680

Signed-off-by: jiefenghuang <[email protected]>
@jiefenghuang jiefenghuang changed the title meta/kv: fix nil value in xattr meta: fix nil value in xattr Feb 25, 2025
@jiefenghuang jiefenghuang changed the title meta: fix nil value in xattr meta: fix empty value in xattr Feb 25, 2025
Signed-off-by: jiefenghuang <[email protected]>
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.20%. Comparing base (e820195) to head (671228f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5696      +/-   ##
==========================================
- Coverage   54.90%   47.20%   -7.71%     
==========================================
  Files         166      131      -35     
  Lines       48600    39629    -8971     
==========================================
- Hits        26684    18705    -7979     
+ Misses      19002    18711     -291     
+ Partials     2914     2213     -701     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: jiefenghuang <[email protected]>
@davies
Copy link
Contributor

davies commented Feb 26, 2025

Redis and SQL database support empty values, so we should only fix that for tkv.

The dump/load should be updated also. This is breaking change, since old version of JuiceFS will not know this special value.

@jiefenghuang
Copy link
Contributor Author

jiefenghuang commented Feb 27, 2025

Redis and SQL database support empty values, so we should only fix that for tkv.

The dump/load should be updated also. This is breaking change, since old version of JuiceFS will not know this special value.

I’ll add the compatibility for load/dump.

SQL also needs to be compatible as value is defined with notnull.
This flag can be removed, the main consideration here is whether it’s better for the three engines to maintain the same behavior.

Value []byte `xorm:"blob notnull"`

@davies
Copy link
Contributor

davies commented Feb 27, 2025

SQL also needs to be compatible as value is defined with notnull. This flag can be removed, the main consideration here is whether it’s better for the three engines to maintain the same behavior.

Value []byte `xorm:"blob notnull"`

Not necessary, this patch is used to workaround a weakness of TiKV.

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.

CI: TestFSOps error: can not set nil value
2 participants