keith-turner commented on a change in pull request #1080: Parallelize
TransactionImpl.readUnread()
URL: https://github.com/apache/fluo/pull/1080#discussion_r340386435
##########
File path:
modules/core/src/main/java/org/apache/fluo/core/impl/TransactionImpl.java
##########
@@ -641,16 +634,8 @@ private void checkForOrphanedReadLocks(CommitData cd,
Map<Bytes, Set<Column>> lo
}
private void checkForOrphanedLocks(CommitData cd) throws Exception {
-
- Map<Bytes, Set<Column>> locksSeen = new HashMap<>();
-
- readUnread(cd, kve -> {
Review comment:
> I don't think I fully understand, is the idea to avoid all write locks or
is it that only a portion of the write locks are actually resolved and we
should avoid that subset?
The goal is to avoid work. If a write lock was resolved for row/column,
then we know there was not a read lock at that row column. Therefore no work
needs to be done attempting to resolve a read lock.
> In terms of keeping track of write locks in ParallelSnapshotScanner would
it be more consistent to add another private Map<Bytes, Set<Column>> similar to
how read locks are tracked? Instead of adding a Consumer.
I think the consumer was used to avoid work. Tracking of write locks is not
always needed. So when its not needed there is no need to update a map. That
is why the code would sometimes pass in a no-op lambda for the consumer.
> Also I didn't know that I could change the public constructors of
SnapshotScanner and ParallelSnapshotScanner, or should I just override them? Is
their any documentation I'd have to update if I change the constructors?
Those classes are not public API, so you can make whatever changes you like.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
