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.

Sign up for GitHub

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

Jump to bottom

Fix a bug in the primitive serializer #949

Merged
merged 6 commits into from
Dec 5, 2017
Merged

Fix a bug in the primitive serializer #949

merged 6 commits into from
Dec 5, 2017

Conversation

no2chem
Copy link
Member

@no2chem no2chem commented Sep 18, 2017

The cache was masking an issue in the primitive serializer where
the primitive type was not being written to the payload. This
bug addresses that issue and adds a unit test.

@corfudb-performance
Copy link
Collaborator

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

*** 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 #949 Graphs

@corfudb-performance
Copy link
Collaborator

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

*** 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 #949 Graphs

@no2chem no2chem added this to the 0.2.0 milestone Dec 4, 2017
@no2chem no2chem self-assigned this Dec 4, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 70.584% when pulling bef9093 on primitiveFix into cbb2c2a on master.

@no2chem no2chem requested a review from annym December 4, 2017 22:22
@corfudb-performance
Copy link
Collaborator

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

*** 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 #949 Graphs

annym
annym reviewed Dec 5, 2017
View reviewed changes
runtime/src/main/java/org/corfudb/util/serializer/PrimitiveSerializer.java Outdated
@@ -40,12 +40,12 @@ public byte getType() {
}

@SuppressWarnings("unchecked")
private static Map<Class, SerializerFunction> getSerializerMap() {
private static Map<Class, Primitives> getSerializerMap() {
ImmutableMap.Builder b = ImmutableMap.<Class, SerializerFunction>builder();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be of the type Primitives

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Looks like the SuppressWarnings plus not assigning to a generic type was hiding that error.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 70.708% when pulling 7c99c5a on primitiveFix into cbb2c2a on master.

@corfudb-performance
Copy link
Collaborator

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

*** 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 #949 Graphs

annym
annym approved these changes Dec 5, 2017
View reviewed changes
Copy link
Collaborator

@annym annym left a comment

Choose a reason for hiding this comment

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

LGTM

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 70.76% when pulling 46abf88 on primitiveFix into d94839a on master.

@no2chem no2chem merged commit 95f853e into master Dec 5, 2017
@no2chem no2chem deleted the primitiveFix branch December 5, 2017 20:25
zalokhan pushed a commit that referenced this pull request Dec 11, 2017
@no2chem
Fix a bug in the primitive serializer (#949)
c588d34 
* Fix primitive serializer

* fix magic numbers

* Fix generic erasure error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annym annym annym approved these changes

@dahliamalkhi dahliamalkhi Awaiting requested review from dahliamalkhi

@Maithem Maithem Awaiting requested review from Maithem

@medhavidhawan medhavidhawan Awaiting requested review from medhavidhawan

Assignees

@no2chem no2chem

Labels
act: ready risk: low typ: bug fix
Projects
None yet
Milestone
0.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@no2chem @corfudb-performance @coveralls @annym