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

fix: Hive max length bug #1878

Merged
merged 7 commits into from
Apr 4, 2024
Merged

fix: Hive max length bug #1878

merged 7 commits into from
Apr 4, 2024

Conversation

murali-shris
Copy link
Member

@murali-shris murali-shris commented Apr 3, 2024

fixes: #1863
- What I did


With this fix, server will return data:null when updating key with length > 255 chars. Have to make fix in server to return correct error message.

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Should we restrict the length to 248 if it doesn't start with "cached:"? Leaves enough for a "cached:"prefix to be added on a caching atServer / client

@murali-shris
Copy link
Member Author

Should we restrict the length to 248 if it doesn't start with "cached:"? Leaves enough for a "cached:"prefix to be added on a caching atServer / client

sure Gary.

@murali-shris
Copy link
Member Author

Should we restrict the length to 248 if it doesn't start with "cached:"? Leaves enough for a "cached:"prefix to be added on a caching atServer / client

sure Gary.

@gkc I had a discussion with @sitaram-kalluri on this.
Let's say the key is @ bob: phone@ alice. This will be stored in alice's keystore. If the key has ttr, then @ bob will cache the key at his end. 255 char limit will be enforced at bob's keystore including cached:
In persistence layer, 255 can be the max limit
in client/server, 248 without cached: and 255 including cached:
is this fine?

@gkc
Copy link
Contributor

gkc commented Apr 4, 2024

Should we restrict the length to 248 if it doesn't start with "cached:"? Leaves enough for a "cached:"prefix to be added on a caching atServer / client

sure Gary.

@gkc I had a discussion with @sitaram-kalluri on this. Let's say the key is @ bob: phone@ alice. This will be stored in alice's keystore. If the key has ttr, then @ bob will cache the key at his end. 255 char limit will be enforced at bob's keystore including cached: In persistence layer, 255 can be the max limit in client/server, 248 without cached: and 255 including cached: is this fine?

Yes I believe so

@murali-shris murali-shris marked this pull request as ready for review April 4, 2024 13:24
@murali-shris murali-shris requested a review from gkc April 4, 2024 13:24
..id = '123')
.build();
var key =
'iujpsefqvdzmtqthrqbaxqszxokaiutvpnbcphcjvjghpdxzdwywfsaowruwafmcudeoarfhuncezjkwbdvprcbujeptisxkjtztxogqqrrnjpqrdsjmcrpmpusrkzaksdfleyzsuarjhsqvxwicxulzqjzcwwjaupxzoqfwenkfonwhxtmwamiyzqqoesnreknrzwxazvykbybafrlwgqsyreudprnakoioqiwoqiwqdebbdwbdywebwydbwrr@alice';
Copy link
Contributor

Choose a reason for hiding this comment

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

For readability, instead of long string of letters unclear as to what length they are, can we instead create and use a function which returns a random string of whatever length is required

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@murali-shris murali-shris merged commit a00a7ff into trunk Apr 4, 2024
21 checks passed
@murali-shris murali-shris deleted the hive_max_length_bug branch April 4, 2024 14:44
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.

Inserting a 253-character key triggers server shutdown upon restart.
2 participants