You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/03/23 15:12:08 UTC
[GitHub] [cassandra] jasonstack opened a new pull request #481: optimize ZCS
file containment check by using file sections
jasonstack opened a new pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] tjake commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
tjake commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r401905753
##########
File path: .circleci/config.yml
##########
@@ -3,10 +3,10 @@ jobs:
j8_jvm_upgrade_dtests:
docker:
- image: spod/cassandra-testing-ubuntu1810-java11-w-dependencies:20190306
- resource_class: medium
Review comment:
I assume you didn't intend to commit this...
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] tjake commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
tjake commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r401903756
##########
File path: src/java/org/apache/cassandra/db/streaming/CassandraOutgoingFile.java
##########
@@ -185,46 +177,18 @@ public boolean shouldStreamEntireSSTable()
if (!DatabaseDescriptor.streamEntireSSTables() || ref.get().getSSTableMetadata().hasLegacyCounterShards)
return false;
- ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(getTableId());
-
- if (cfs == null)
- return false;
-
- AbstractCompactionStrategy compactionStrategy = cfs.getCompactionStrategyManager()
- .getCompactionStrategyFor(ref.get());
-
- if (compactionStrategy instanceof LeveledCompactionStrategy)
- return contained(normalizedRanges, ref.get());
-
- return false;
+ return contained(sections, ref.get());
}
@VisibleForTesting
- public boolean contained(List<Range<Token>> normalizedRanges, SSTableReader sstable)
+ public boolean contained(List<SSTableReader.PartitionPositionBounds> sections, SSTableReader sstable)
{
- if (isFullyContained != null)
- return isFullyContained;
-
- isFullyContained = computeContainment(normalizedRanges, sstable);
- return isFullyContained;
- }
-
- private boolean computeContainment(List<Range<Token>> normalizedRanges, SSTableReader sstable)
- {
- if (normalizedRanges == null)
+ if (sections == null || sections.isEmpty())
return false;
- RangeOwnHelper rangeOwnHelper = new RangeOwnHelper(normalizedRanges);
- try (KeyIterator iter = new KeyIterator(sstable.descriptor, sstable.metadata()))
- {
- while (iter.hasNext())
- {
- DecoratedKey key = iter.next();
- if (!rangeOwnHelper.check(key))
- return false;
- }
- }
- return true;
+ // if transfer sections contain entire sstable
+ long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();
Review comment:
These sections should be normalized, so you will only have 1 when its the full sstable right?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jasonstack commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
jasonstack commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r402760415
##########
File path: src/java/org/apache/cassandra/db/streaming/CassandraOutgoingFile.java
##########
@@ -185,46 +177,18 @@ public boolean shouldStreamEntireSSTable()
if (!DatabaseDescriptor.streamEntireSSTables() || ref.get().getSSTableMetadata().hasLegacyCounterShards)
return false;
- ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(getTableId());
-
- if (cfs == null)
- return false;
-
- AbstractCompactionStrategy compactionStrategy = cfs.getCompactionStrategyManager()
- .getCompactionStrategyFor(ref.get());
-
- if (compactionStrategy instanceof LeveledCompactionStrategy)
- return contained(normalizedRanges, ref.get());
-
- return false;
+ return contained(sections, ref.get());
}
@VisibleForTesting
- public boolean contained(List<Range<Token>> normalizedRanges, SSTableReader sstable)
+ public boolean contained(List<SSTableReader.PartitionPositionBounds> sections, SSTableReader sstable)
{
- if (isFullyContained != null)
- return isFullyContained;
-
- isFullyContained = computeContainment(normalizedRanges, sstable);
- return isFullyContained;
- }
-
- private boolean computeContainment(List<Range<Token>> normalizedRanges, SSTableReader sstable)
- {
- if (normalizedRanges == null)
+ if (sections == null || sections.isEmpty())
return false;
- RangeOwnHelper rangeOwnHelper = new RangeOwnHelper(normalizedRanges);
- try (KeyIterator iter = new KeyIterator(sstable.descriptor, sstable.metadata()))
- {
- while (iter.hasNext())
- {
- DecoratedKey key = iter.next();
- if (!rangeOwnHelper.check(key))
- return false;
- }
- }
- return true;
+ // if transfer sections contain entire sstable
+ long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();
Review comment:
I think there can be more than 1 section if ranges don't overlap..
eg. transfer ranges (0, 1000], (2000, 3000] and sstable has token 100, 200, 2100, 2200.. there will be two sections..
I have reverted by to `long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();` WDYT?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jasonstack commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
jasonstack commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r402760415
##########
File path: src/java/org/apache/cassandra/db/streaming/CassandraOutgoingFile.java
##########
@@ -185,46 +177,18 @@ public boolean shouldStreamEntireSSTable()
if (!DatabaseDescriptor.streamEntireSSTables() || ref.get().getSSTableMetadata().hasLegacyCounterShards)
return false;
- ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(getTableId());
-
- if (cfs == null)
- return false;
-
- AbstractCompactionStrategy compactionStrategy = cfs.getCompactionStrategyManager()
- .getCompactionStrategyFor(ref.get());
-
- if (compactionStrategy instanceof LeveledCompactionStrategy)
- return contained(normalizedRanges, ref.get());
-
- return false;
+ return contained(sections, ref.get());
}
@VisibleForTesting
- public boolean contained(List<Range<Token>> normalizedRanges, SSTableReader sstable)
+ public boolean contained(List<SSTableReader.PartitionPositionBounds> sections, SSTableReader sstable)
{
- if (isFullyContained != null)
- return isFullyContained;
-
- isFullyContained = computeContainment(normalizedRanges, sstable);
- return isFullyContained;
- }
-
- private boolean computeContainment(List<Range<Token>> normalizedRanges, SSTableReader sstable)
- {
- if (normalizedRanges == null)
+ if (sections == null || sections.isEmpty())
return false;
- RangeOwnHelper rangeOwnHelper = new RangeOwnHelper(normalizedRanges);
- try (KeyIterator iter = new KeyIterator(sstable.descriptor, sstable.metadata()))
- {
- while (iter.hasNext())
- {
- DecoratedKey key = iter.next();
- if (!rangeOwnHelper.check(key))
- return false;
- }
- }
- return true;
+ // if transfer sections contain entire sstable
+ long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();
Review comment:
I think there can be more than 1 section if ranges don't overlap..
eg. transfer ranges (0, 1000], (2000, 3000] and sstable has token 100, 200, 2100, 2200.. there will be two sections..
I have reverted to `long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();` WDYT?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jasonstack commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
jasonstack commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r401918700
##########
File path: src/java/org/apache/cassandra/db/streaming/CassandraOutgoingFile.java
##########
@@ -185,46 +177,18 @@ public boolean shouldStreamEntireSSTable()
if (!DatabaseDescriptor.streamEntireSSTables() || ref.get().getSSTableMetadata().hasLegacyCounterShards)
return false;
- ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(getTableId());
-
- if (cfs == null)
- return false;
-
- AbstractCompactionStrategy compactionStrategy = cfs.getCompactionStrategyManager()
- .getCompactionStrategyFor(ref.get());
-
- if (compactionStrategy instanceof LeveledCompactionStrategy)
- return contained(normalizedRanges, ref.get());
-
- return false;
+ return contained(sections, ref.get());
}
@VisibleForTesting
- public boolean contained(List<Range<Token>> normalizedRanges, SSTableReader sstable)
+ public boolean contained(List<SSTableReader.PartitionPositionBounds> sections, SSTableReader sstable)
{
- if (isFullyContained != null)
- return isFullyContained;
-
- isFullyContained = computeContainment(normalizedRanges, sstable);
- return isFullyContained;
- }
-
- private boolean computeContainment(List<Range<Token>> normalizedRanges, SSTableReader sstable)
- {
- if (normalizedRanges == null)
+ if (sections == null || sections.isEmpty())
return false;
- RangeOwnHelper rangeOwnHelper = new RangeOwnHelper(normalizedRanges);
- try (KeyIterator iter = new KeyIterator(sstable.descriptor, sstable.metadata()))
- {
- while (iter.hasNext())
- {
- DecoratedKey key = iter.next();
- if (!rangeOwnHelper.check(key))
- return false;
- }
- }
- return true;
+ // if transfer sections contain entire sstable
+ long transferLength = sections.stream().mapToLong(p -> p.upperPosition - p.lowerPosition).sum();
Review comment:
didn't realize `Range.normalize` will combine overlapping ranges.. updated here 188ed6d
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jasonstack closed pull request #481: optimize ZCS file
containment check by using file sections
Posted by GitBox <gi...@apache.org>.
jasonstack closed pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jasonstack commented on a change in pull request #481:
optimize ZCS file containment check by using file sections
Posted by GitBox <gi...@apache.org>.
jasonstack commented on a change in pull request #481: optimize ZCS file containment check by using file sections
URL: https://github.com/apache/cassandra/pull/481#discussion_r401915940
##########
File path: .circleci/config.yml
##########
@@ -3,10 +3,10 @@ jobs:
j8_jvm_upgrade_dtests:
docker:
- image: spod/cassandra-testing-ubuntu1810-java11-w-dependencies:20190306
- resource_class: medium
Review comment:
right, not to commit. it's to speed up CI during our circle-ci trial period..
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org