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

Optimize Read Channels #940

Merged
merged 1 commit into from
Dec 13, 2017
Merged

Optimize Read Channels #940

merged 1 commit into from
Dec 13, 2017

Conversation

Maithem
Copy link
Contributor

@Maithem Maithem commented Sep 11, 2017

Use a single channel per segment for uncached reads.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ba6a205 on optReads into ** on master**.

@corfudb-performance
Copy link
Collaborator

Results automatically generated by CorfuDB Benchmark Framework to assess the performance of this pull request for commit ba6a205.

*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 1 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 5 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 10 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Iterator workload, 1 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Iterator workload, 5 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Iterator workload, 10 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 1 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 5 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 10 threads, Disk mode

An interactive dashboard with Pull Request Performance Metrics for ALL cluster types and numbers of threads in run, is available at:
Pull Request #940 Graphs

@no2chem no2chem added this to the 0.2.0 milestone Dec 5, 2017
Copy link
Member

@no2chem no2chem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Use a single channel per segment for to read uncached entries.
@Maithem
Copy link
Contributor Author

Maithem commented Dec 13, 2017

@no2chem Rebased.

try {
channel.force(true);
channel.close();
channel = null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR Remove this useless assignment to local variable "channel". rule

@NonNull
String fileName;

private Map<Long, AddressMetaData> knownAddresses = new ConcurrentHashMap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR Remove the "knownAddresses" field and declare it as a local variable in the relevant methods. rule

@corfu-repo
Copy link
Collaborator

SonarQube analysis reported 5 issues

  • MAJOR 1 major
  • MINOR 4 minor

Watch the comments in this conversation to review them.

1 extra issue

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MINOR StreamLogFiles.java#L245: Remove this unnecessary cast to "Collection". rule

String fileName;

private Map<Long, AddressMetaData> knownAddresses = new ConcurrentHashMap();
private Set<Long> trimmedAddresses = Collections.newSetFromMap(new ConcurrentHashMap<>());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR Remove the "trimmedAddresses" field and declare it as a local variable in the relevant methods. rule


private Map<Long, AddressMetaData> knownAddresses = new ConcurrentHashMap();
private Set<Long> trimmedAddresses = Collections.newSetFromMap(new ConcurrentHashMap<>());
private Set<Long> pendingTrims = Collections.newSetFromMap(new ConcurrentHashMap<>());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR Remove the "pendingTrims" field and declare it as a local variable in the relevant methods. rule

@codecov
Copy link

codecov bot commented Dec 13, 2017

Codecov Report

Merging #940 into master will increase coverage by 0.16%.
The diff coverage is 88.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #940      +/-   ##
==========================================
+ Coverage   66.85%   67.01%   +0.16%     
==========================================
  Files         211      212       +1     
  Lines        9856     9853       -3     
  Branches      986      983       -3     
==========================================
+ Hits         6589     6603      +14     
+ Misses       2887     2871      -16     
+ Partials      380      379       -1
Impacted Files Coverage Δ
.../org/corfudb/infrastructure/log/SegmentHandle.java 84.37% <84.37%> (ø)
...org/corfudb/infrastructure/log/StreamLogFiles.java 83.62% <92.59%> (+0.03%) ⬆️
...org/corfudb/runtime/view/LayoutManagementView.java 50% <0%> (-2.35%) ⬇️
...rg/corfudb/runtime/object/VersionLockedObject.java 89.67% <0%> (+0.54%) ⬆️
...main/java/org/corfudb/runtime/view/LayoutView.java 84.84% <0%> (+0.75%) ⬆️
.../org/corfudb/protocols/wireprotocol/IMetadata.java 86.95% <0%> (+2.89%) ⬆️
...fudb/infrastructure/orchestrator/Orchestrator.java 76.81% <0%> (+4.34%) ⬆️
.../org/corfudb/runtime/object/CorfuCompileProxy.java 68.78% <0%> (+6.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 287ada9...5db09d7. Read the comment docs.

@corfudb-performance
Copy link
Collaborator

Results automatically generated by CorfuDB Benchmark Framework to assess the performance of this pull request for commit 5db09d7.

*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 1 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 5 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx+Scan workload, 10 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 1 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 5 threads, Disk mode
*** 0.0% transaction FAILURE rate for NonConflictingTx workload, 10 threads, Disk mode

An interactive dashboard with Pull Request Performance Metrics for ALL cluster types and numbers of threads in run, is available at:
Pull Request #940 Graphs

@Maithem Maithem merged commit 4acfbe4 into master Dec 13, 2017
@Maithem Maithem deleted the optReads branch December 13, 2017 07:30
@Maithem Maithem restored the optReads branch December 14, 2017 05:33
@Maithem Maithem mentioned this pull request Dec 14, 2017
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants