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

Issue 4967: (SLTS) Add createWithContent overload that creates object and writes to it in a single call. #5455

Merged
merged 10 commits into from
Feb 4, 2021
Merged

Issue 4967: (SLTS) Add createWithContent overload that creates object and writes to it in a single call. #5455

merged 10 commits into from
Feb 4, 2021

Conversation

sachin-j-joshi
Copy link
Contributor

Signed-off-by: Sachin Joshi sachin.joshi@emc.com

Change log description
Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Purpose of the change
Fixes #4967

What the code does
Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation accordingly.

How to verify it
Unit tests and other tests with Extended S3 with no append mode should pass.

@sachin-j-joshi sachin-j-joshi added area/lts kind/enhancement Improvements that should be made labels Jan 4, 2021
@sachin-j-joshi sachin-j-joshi changed the title Issue 4967: (SLTS) Add createWithContexnt overload that create object and writes to it in a single call. Issue 4967: (SLTS) Add createWithContent overload that create object and writes to it in a single call. Jan 4, 2021
@sachin-j-joshi sachin-j-joshi changed the title Issue 4967: (SLTS) Add createWithContent overload that create object and writes to it in a single call. Issue 4967: (SLTS) Add createWithContent overload that creates object and writes to it in a single call. Jan 4, 2021
@sachin-j-joshi sachin-j-joshi force-pushed the issue-4967-SLTS-optimize-chunk-creation branch from 59e980e to 370ef37 Compare January 12, 2021 00:54
@sachin-j-joshi
Issue 4967: (SLTS) Optimize chunk addition by combining create follow…
c277a3c 
…ed by write in a single call.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
@sachin-j-joshi
Issue 4967: (SLTS) More checks.
9e6ca65 
Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
@sachin-j-joshi
Issue 4967: (SLTS) Remove redundant check.
a4c4f81 
Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
@sachin-j-joshi sachin-j-joshi force-pushed the issue-4967-SLTS-optimize-chunk-creation branch from 370ef37 to a4c4f81 Compare January 13, 2021 23:29
eolivelli
eolivelli approved these changes Jan 14, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

andreipaduroiu
andreipaduroiu requested changes Jan 21, 2021
View reviewed changes
bindings/src/main/java/io/pravega/storage/extendeds3/ExtendedS3ChunkStorage.java Outdated Show resolved Hide resolved
...st/src/test/java/io/pravega/segmentstore/server/host/NonAppendExtendedS3IntegrationTest.java Outdated Show resolved Hide resolved
.../storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/AsyncBaseChunkStorage.java Show resolved Hide resolved
.../storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/AsyncBaseChunkStorage.java Outdated Show resolved Hide resolved
...store/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/BaseChunkStorage.java Outdated Show resolved Hide resolved
...store/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/BaseChunkStorage.java Show resolved Hide resolved
segmentstore/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/ChunkStorage.java Outdated Show resolved Hide resolved
...entstore/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/SystemJournal.java Show resolved Hide resolved
...entstore/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/SystemJournal.java Show resolved Hide resolved
...entstore/storage/src/main/java/io/pravega/segmentstore/storage/chunklayer/SystemJournal.java Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 21, 2021 •
edited

Codecov Report

Merging #5455 (fc1eedd) into master (d1eb534) will increase coverage by 0.04%.
The diff coverage is 89.55%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5455      +/-   ##
============================================
+ Coverage     84.71%   84.76%   +0.04%     
- Complexity    13870    13886      +16     
============================================
  Files           917      917              
  Lines         51359    51407      +48     
  Branches       5301     5307       +6     
============================================
+ Hits          43510    43576      +66     
+ Misses         4957     4942      -15     
+ Partials       2892     2889       -3
Impacted Files Coverage Δ Complexity Δ
...mentstore/storage/chunklayer/BaseChunkStorage.java 84.61% <57.14%> (-10.13%) 17.00 <2.00> (+2.00) ⬇️
...ega/storage/extendeds3/ExtendedS3ChunkStorage.java 88.23% <86.66%> (-0.34%) 39.00 <6.00> (+1.00) ⬇️
...segmentstore/storage/chunklayer/SystemJournal.java 93.50% <90.00%> (+0.11%) 50.00 <4.00> (+1.00)
...age/extendeds3/ExtendedS3SimpleStorageFactory.java 85.71% <100.00%> (ø) 2.00 <0.00> (ø)
...tore/storage/chunklayer/AsyncBaseChunkStorage.java 82.84% <100.00%> (+1.02%) 39.00 <4.00> (+3.00)
...egmentstore/storage/chunklayer/WriteOperation.java 94.97% <100.00%> (+0.18%) 63.00 <1.00> (+3.00)
...hared/security/crypto/StrongPasswordProcessor.java 87.50% <0.00%> (-6.25%) 7.00% <0.00%> (-1.00%)
...server/attributes/ContainerAttributeIndexImpl.java 79.10% <0.00%> (-2.99%) 17.00% <0.00%> (-2.00%)
...segmentstore/storage/chunklayer/ReadOperation.java 88.69% <0.00%> (-1.79%) 39.00% <0.00%> (-1.00%)
...ega/segmentstore/server/logs/DataFrameBuilder.java 81.08% <0.00%> (-1.36%) 12.00% <0.00%> (-2.00%)
... and 16 more

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 d1eb534...fc1eedd. Read the comment docs.

@sachin-j-joshi
Merge branch 'master' of https://github.com/pravega/pravega into issu…
5b8be4f 
…e-4967-SLTS-optimize-chunk-creation

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

# Conflicts:
#	bindings/src/main/java/io/pravega/storage/extendeds3/ExtendedS3ChunkStorage.java
#	bindings/src/test/java/io/pravega/storage/extendeds3/S3FileSystemImpl.java
#	bindings/src/test/java/io/pravega/storage/extendeds3/S3ProxyImpl.java
@sachin-j-joshi
Issue 4967: (SLTS) Address review comments.
575a686 
Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
@sachin-j-joshi
Merge branch 'master' of https://github.com/pravega/pravega into issu…
42cddcf 
…e-4967-SLTS-optimize-chunk-creation
@andreipaduroiu andreipaduroiu merged commit 6c39ead into pravega:master Feb 4, 2021
tkaitchuck pushed a commit to tkaitchuck/pravega-1 that referenced this pull request Feb 15, 2021
@sachin-j-joshi @andreipaduroiu @tkaitchuck
Issue 4967: (SLTS) Add createWithContent overload that creates object…
e48e1ee 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
tkaitchuck pushed a commit to tkaitchuck/pravega-1 that referenced this pull request Feb 15, 2021
@sachin-j-joshi @andreipaduroiu @tkaitchuck
Issue 4967: (SLTS) Add createWithContent overload that creates object…
382e5fa 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
Signed-off-by: Tom Kaitchuck <tom.kaitchuck@emc.com>
co-jo pushed a commit to co-jo/pravega that referenced this pull request Feb 16, 2021
@sachin-j-joshi @andreipaduroiu
Issue 4967: (SLTS) Add createWithContent overload that creates object…
0be4672 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
sachin-j-joshi added a commit to sachin-j-joshi/pravega that referenced this pull request Feb 23, 2021
@sachin-j-joshi @andreipaduroiu
Issue 4967: (SLTS) Add createWithContent overload that creates object…
1f9d19c 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
@sachin-j-joshi sachin-j-joshi mentioned this pull request Feb 24, 2021
Merged
co-jo pushed a commit to co-jo/pravega that referenced this pull request Mar 1, 2021
@sachin-j-joshi @andreipaduroiu
Issue 4967: (SLTS) Add createWithContent overload that creates object…
3075000 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
co-jo pushed a commit to co-jo/pravega that referenced this pull request Mar 9, 2021
@sachin-j-joshi @andreipaduroiu
Issue 4967: (SLTS) Add createWithContent overload that creates object…
c53aa6e 
… and writes to it in a single call. (pravega#5455)

Provide another overload to ChunkStorage.create that takes initial data to write and writes data in a single call.
In addition it avoids using Range header for put or post which may not be supported by all S3 compliant storages.
Fix mock S3 implementation and add more tests specifically for Extended S3 with no append mode.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>

Co-authored-by: Andrei Paduroiu <andreipaduroiu@users.noreply.github.com>
sachin-j-joshi added a commit that referenced this pull request Mar 19, 2021
@sachin-j-joshi
Issue 5780: (SLTS) - Cherry pick bug fixes to 0.9 (#5782)
cd7462b 
Cherry-picks following changes 0.9

Issue 5518: (SLTS) Fix Integer overflow. (#5520)
Issue 5606: BoundedInputStream::markSupported() should always return false.. (#5615)
Issue 5597 : ExtendedS3ChunkStorage and ExtendedS3Storage should close S3Client. (#5598)
Issue 5535: (SegmentStore) Refactoring ExtendedS3 Test Mocks (#5537)
Issue 5456: (SLTS) - Handle possible failure during deletion of metadata keys can cause version mismatch in next attempt (#5457)
Issue 5458: (SLTS) For read operation, read all chunks in parallel. (#5459)
Issue 4967: (SLTS) Add createWithContent overload that creates object and writes to it in a single call. (#5455)
Issue 5664: SLTS - fix possible thread visibility issues.. (#5665)
Issue 5475: Eliminate unnecessary pessimistic checks. (#5662)
Issue 5570: (SLTS) AsyncBaseChunkStorage latency should not include time spent in waiting to be executed. (#5661)
Issue 4967: (SLTS) No need to check last chunk length for claimOwnership for no append mode. (#5663)
Issue 5673: (SLTS) Fix wrong GC config (#5674)
Issue 5737: (SLTS) Improve metrics (#5746)
Issue 5460: (SLTS) Add read index virtual block entries to metadata. (#5461)
Issue 5772: (SLTS) - BaseMetadataStore does not evict entries from buffer. (#5773)
Issue 5798: (SLTS) Fix close in GarbageCollector, ExtendedS3ChunkStorage and HDFSChunkStorage. (#5800)
Issue 5788: (SLTS) Remove ACL operations from ExtendedS3ChunkStorage (#5790)
Issue 5808: SLTS - Fix Preconditions checks to include enough information in message. (#5809)
Issue 5853: SLTS - BaseMetadataStore.get does not return deep copy when loading from store. #5858
Issue 5866: (SLTS) ReadIndexCache stats not reported. (#5867)


Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
@sachin-j-joshi sachin-j-joshi deleted the issue-4967-SLTS-optimize-chunk-creation branch August 23, 2021 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaulGracia RaulGracia RaulGracia left review comments

@eolivelli eolivelli eolivelli approved these changes

@andreipaduroiu andreipaduroiu andreipaduroiu approved these changes

Assignees
No one assigned
Labels
area/lts kind/enhancement Improvements that should be made
Projects
None yet
Milestone
No milestone
4 participants
@sachin-j-joshi @RaulGracia @eolivelli @andreipaduroiu