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

Fate lock code use of byte[] obfuscates the simple data being stored #5264

Closed
keith-turner opened this issue Jan 15, 2025 · 1 comment · Fixed by #5298
Closed

Fate lock code use of byte[] obfuscates the simple data being stored #5264

keith-turner opened this issue Jan 15, 2025 · 1 comment · Fixed by #5298
Assignees
Milestone

Comments

@keith-turner
Copy link
Contributor

The fate code for locking tables and namespaces is spread across three classes o.a.a.manager.tableOps.Utils, DistributedReadWriteLock, and FateLock. The data for a fate lock consist of two fields the lock type(read or write) and a fate id. Looking at the code and determining it stores these two bits of data is time consuming because the code passes around byte arrays that have that information encoded. If the code be refactored to use concrete types (FateId and DistributedReadWriteLock.LockType) deeper in the code and defer serialization to byte[] till much later in the code then this would make it much easier to understand and maintain the code and keep it correct.

@keith-turner keith-turner added this to the 4.0.0 milestone Jan 15, 2025
@keith-turner keith-turner changed the title Fate lock code use of byte[] obfuscates the simple data is being stored Fate lock code use of byte[] obfuscates the simple data that is being stored Jan 15, 2025
@keith-turner keith-turner changed the title Fate lock code use of byte[] obfuscates the simple data that is being stored Fate lock code use of byte[] obfuscates the simple data being stored Jan 15, 2025
@cshannon
Copy link
Contributor

I can go ahead and take a look at this

@cshannon cshannon self-assigned this Jan 24, 2025
cshannon added a commit to cshannon/accumulo that referenced this issue Jan 31, 2025
Instead of passing around byte arrays for DistributedReadWriteLock and
related classes we can use LockType and FateId to make the code easier
to work with and to undertand as those values are serialized as part of
the lock. A new FateLockEntry type has been created which is passed
around instead and that is serialized/deserialized into the correct byte
array format when reading/writing to zookeeeper.

This closes apache#5264
cshannon added a commit to cshannon/accumulo that referenced this issue Jan 31, 2025
Instead of passing around byte arrays for DistributedReadWriteLock and
related classes we can use LockType and FateId to make the code easier
to work with and to undertand as those values are serialized as part of
the lock. A new FateLockEntry type has been created which is passed
around instead and that is serialized/deserialized into the correct byte
array format when reading/writing to zookeeeper.

This closes apache#5264
cshannon added a commit to cshannon/accumulo that referenced this issue Jan 31, 2025
Instead of passing around byte arrays for DistributedReadWriteLock and
related classes we can use LockType and FateId to make the code easier
to work with and to understand as those values are serialized as part of
the lock. A new FateLockEntry type has been created which is passed
around instead and that is serialized/deserialized into the correct byte
array format when reading/writing to zookeeeper.

This closes apache#5264
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 a pull request may close this issue.

2 participants