You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "cshannon (via GitHub)" <gi...@apache.org> on 2024/02/23 16:03:29 UTC
[PR] Replace boolean with enum for table locks [accumulo]
cshannon opened a new pull request, #4297:
URL: https://github.com/apache/accumulo/pull/4297
Instead of using a boolean value for obtaining table locks, use an enum which makes the code more readable and makes it easier to search and catch errors with the lock type.
This closes #4276
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace boolean with enum for table locks [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #4297:
URL: https://github.com/apache/accumulo/pull/4297#issuecomment-1961594906
This current PR is for main, I'll fix the conflicts when merging into elasticity after this is merged to main
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace boolean with enum for table locks in 3.1 [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon merged PR #4297:
URL: https://github.com/apache/accumulo/pull/4297
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace boolean with enum for table locks in 3.1 [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4297:
URL: https://github.com/apache/accumulo/pull/4297#discussion_r1503057721
##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java:
##########
@@ -163,27 +161,30 @@ public static void unreserveHdfsDirectory(Manager env, String directory, long ti
}
private static Lock getLock(ServerContext context, AbstractId<?> id, long tid,
- boolean writeLock) {
+ LockType lockType) {
byte[] lockData = FastFormat.toZeroPaddedHex(tid);
var fLockPath =
FateLock.path(context.getZooKeeperRoot() + Constants.ZTABLE_LOCKS + "/" + id.canonical());
FateLock qlock = new FateLock(context.getZooReaderWriter(), fLockPath);
DistributedLock lock = DistributedReadWriteLock.recoverLock(qlock, lockData);
if (lock != null) {
-
// Validate the recovered lock type
- boolean isWriteLock = lock.getType() == LockType.WRITE;
- if (writeLock != isWriteLock) {
+ if (!lock.getType().equals(lockType)) {
Review Comment:
If these enums should be able to use `!=`
```suggestion
if (lock.getType()!= lockType) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace boolean with enum for table locks [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #4297:
URL: https://github.com/apache/accumulo/pull/4297#issuecomment-1961600616
Actually in elasticity virtually everything is a conflict so it will be simpler to merge this forward as an empty commit and then just do a separate PR for elasticity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org