Committed by
Gerrit Code Review
Fix for NPE in entry removal updates.
Reference: ONOS-1785 Change-Id: I146e6cbb39b2fdc36cec3e2df1b883f578f0200a
Showing
1 changed file
with
30 additions
and
13 deletions
| ... | @@ -30,7 +30,6 @@ import org.onosproject.store.service.DatabaseUpdate; | ... | @@ -30,7 +30,6 @@ import org.onosproject.store.service.DatabaseUpdate; |
| 30 | import org.onosproject.store.service.Transaction; | 30 | import org.onosproject.store.service.Transaction; |
| 31 | import org.onosproject.store.service.Versioned; | 31 | import org.onosproject.store.service.Versioned; |
| 32 | import org.onosproject.store.service.DatabaseUpdate.Type; | 32 | import org.onosproject.store.service.DatabaseUpdate.Type; |
| 33 | - | ||
| 34 | import com.google.common.base.Objects; | 33 | import com.google.common.base.Objects; |
| 35 | import com.google.common.collect.ImmutableList; | 34 | import com.google.common.collect.ImmutableList; |
| 36 | import com.google.common.collect.ImmutableSet; | 35 | import com.google.common.collect.ImmutableSet; |
| ... | @@ -57,7 +56,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -57,7 +56,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 57 | * The presence of a entry in this map indicates that element is | 56 | * The presence of a entry in this map indicates that element is |
| 58 | * participating in a transaction and is currently locked for updates. | 57 | * participating in a transaction and is currently locked for updates. |
| 59 | */ | 58 | */ |
| 60 | - private Map<String, Map<String, Pair<Long, byte[]>>> locks; | 59 | + private Map<String, Map<String, Update>> locks; |
| 61 | 60 | ||
| 62 | @Initializer | 61 | @Initializer |
| 63 | @Override | 62 | @Override |
| ... | @@ -275,7 +274,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -275,7 +274,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 275 | return tables.computeIfAbsent(tableName, name -> Maps.newConcurrentMap()); | 274 | return tables.computeIfAbsent(tableName, name -> Maps.newConcurrentMap()); |
| 276 | } | 275 | } |
| 277 | 276 | ||
| 278 | - private Map<String, Pair<Long, byte[]>> getLockMap(String tableName) { | 277 | + private Map<String, Update> getLockMap(String tableName) { |
| 279 | return locks.computeIfAbsent(tableName, name -> Maps.newConcurrentMap()); | 278 | return locks.computeIfAbsent(tableName, name -> Maps.newConcurrentMap()); |
| 280 | } | 279 | } |
| 281 | 280 | ||
| ... | @@ -305,18 +304,18 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -305,18 +304,18 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 305 | } | 304 | } |
| 306 | 305 | ||
| 307 | private void doProvisionalUpdate(DatabaseUpdate update, long transactionId) { | 306 | private void doProvisionalUpdate(DatabaseUpdate update, long transactionId) { |
| 308 | - Map<String, Pair<Long, byte[]>> lockMap = getLockMap(update.tableName()); | 307 | + Map<String, Update> lockMap = getLockMap(update.tableName()); |
| 309 | switch (update.type()) { | 308 | switch (update.type()) { |
| 310 | case PUT: | 309 | case PUT: |
| 311 | case PUT_IF_ABSENT: | 310 | case PUT_IF_ABSENT: |
| 312 | case PUT_IF_VERSION_MATCH: | 311 | case PUT_IF_VERSION_MATCH: |
| 313 | case PUT_IF_VALUE_MATCH: | 312 | case PUT_IF_VALUE_MATCH: |
| 314 | - lockMap.put(update.key(), Pair.of(transactionId, update.value())); | 313 | + lockMap.put(update.key(), new Update(transactionId, update.value())); |
| 315 | break; | 314 | break; |
| 316 | case REMOVE: | 315 | case REMOVE: |
| 317 | case REMOVE_IF_VERSION_MATCH: | 316 | case REMOVE_IF_VERSION_MATCH: |
| 318 | case REMOVE_IF_VALUE_MATCH: | 317 | case REMOVE_IF_VALUE_MATCH: |
| 319 | - lockMap.put(update.key(), null); | 318 | + lockMap.put(update.key(), new Update(transactionId, null)); |
| 320 | break; | 319 | break; |
| 321 | default: | 320 | default: |
| 322 | throw new IllegalStateException("Unsupported type: " + update.type()); | 321 | throw new IllegalStateException("Unsupported type: " + update.type()); |
| ... | @@ -327,8 +326,8 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -327,8 +326,8 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 327 | String tableName = update.tableName(); | 326 | String tableName = update.tableName(); |
| 328 | String key = update.key(); | 327 | String key = update.key(); |
| 329 | Type type = update.type(); | 328 | Type type = update.type(); |
| 330 | - Pair<Long, byte[]> provisionalUpdate = getLockMap(tableName).get(key); | 329 | + Update provisionalUpdate = getLockMap(tableName).get(key); |
| 331 | - if (Objects.equal(transactionId, provisionalUpdate.getLeft())) { | 330 | + if (Objects.equal(transactionId, provisionalUpdate.transactionId())) { |
| 332 | getLockMap(tableName).remove(key); | 331 | getLockMap(tableName).remove(key); |
| 333 | } else { | 332 | } else { |
| 334 | return; | 333 | return; |
| ... | @@ -339,7 +338,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -339,7 +338,7 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 339 | case PUT_IF_ABSENT: | 338 | case PUT_IF_ABSENT: |
| 340 | case PUT_IF_VERSION_MATCH: | 339 | case PUT_IF_VERSION_MATCH: |
| 341 | case PUT_IF_VALUE_MATCH: | 340 | case PUT_IF_VALUE_MATCH: |
| 342 | - put(tableName, key, provisionalUpdate.getRight()); | 341 | + put(tableName, key, provisionalUpdate.value()); |
| 343 | break; | 342 | break; |
| 344 | case REMOVE: | 343 | case REMOVE: |
| 345 | case REMOVE_IF_VERSION_MATCH: | 344 | case REMOVE_IF_VERSION_MATCH: |
| ... | @@ -354,18 +353,18 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -354,18 +353,18 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 354 | private void undoProvisionalUpdate(DatabaseUpdate update, long transactionId) { | 353 | private void undoProvisionalUpdate(DatabaseUpdate update, long transactionId) { |
| 355 | String tableName = update.tableName(); | 354 | String tableName = update.tableName(); |
| 356 | String key = update.key(); | 355 | String key = update.key(); |
| 357 | - Pair<Long, byte[]> provisionalUpdate = getLockMap(tableName).get(key); | 356 | + Update provisionalUpdate = getLockMap(tableName).get(key); |
| 358 | if (provisionalUpdate == null) { | 357 | if (provisionalUpdate == null) { |
| 359 | return; | 358 | return; |
| 360 | } | 359 | } |
| 361 | - if (Objects.equal(transactionId, provisionalUpdate.getLeft())) { | 360 | + if (Objects.equal(transactionId, provisionalUpdate.transactionId())) { |
| 362 | getLockMap(tableName).remove(key); | 361 | getLockMap(tableName).remove(key); |
| 363 | } | 362 | } |
| 364 | } | 363 | } |
| 365 | 364 | ||
| 366 | private boolean isLockedByAnotherTransaction(String tableName, String key, long transactionId) { | 365 | private boolean isLockedByAnotherTransaction(String tableName, String key, long transactionId) { |
| 367 | - Pair<Long, byte[]> update = getLockMap(tableName).get(key); | 366 | + Update update = getLockMap(tableName).get(key); |
| 368 | - return update != null && !Objects.equal(transactionId, update.getLeft()); | 367 | + return update != null && !Objects.equal(transactionId, update.transactionId()); |
| 369 | } | 368 | } |
| 370 | 369 | ||
| 371 | private boolean isLockedForUpdates(String tableName, String key) { | 370 | private boolean isLockedForUpdates(String tableName, String key) { |
| ... | @@ -375,4 +374,22 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { | ... | @@ -375,4 +374,22 @@ public class DefaultDatabaseState implements DatabaseState<String, byte[]> { |
| 375 | private boolean areTransactionsInProgress(String tableName) { | 374 | private boolean areTransactionsInProgress(String tableName) { |
| 376 | return !getLockMap(tableName).isEmpty(); | 375 | return !getLockMap(tableName).isEmpty(); |
| 377 | } | 376 | } |
| 377 | + | ||
| 378 | + private class Update { | ||
| 379 | + private final long transactionId; | ||
| 380 | + private final byte[] value; | ||
| 381 | + | ||
| 382 | + public Update(long txId, byte[] value) { | ||
| 383 | + this.transactionId = txId; | ||
| 384 | + this.value = value; | ||
| 385 | + } | ||
| 386 | + | ||
| 387 | + public long transactionId() { | ||
| 388 | + return this.transactionId; | ||
| 389 | + } | ||
| 390 | + | ||
| 391 | + public byte[] value() { | ||
| 392 | + return this.value; | ||
| 393 | + } | ||
| 394 | + } | ||
| 378 | } | 395 | } | ... | ... |
-
Please register or login to post a comment