[GitHub] chetanmeh opened a new pull request #3777: Avoid converting ByteString to bytes in attachment inlining flow

[GitBox <gi...@apache.org>]

chetanmeh opened a new pull request #3777: Avoid converting ByteString to bytes in attachment inlining flow
URL: https://github.com/apache/incubator-openwhisk/pull/3777
 
 
   This PR modifies the current attachment inlining logic to work at `ByteString` level and avoid converting ByteString to bytes and back. This fixes #3770
   
   ## Description
   
   So far the inlining logic was converting a stream of `ByteString` to byte array and then working at byte level. This was causing the upload to slow down and also reverse conversion of stream of bytes to `ByteString` was not working as expected.
   
   Instead of doing that it now uses approach mentioned [here](https://groups.google.com/forum/#!topic/akka-user/FdQAAzcwrqM) and works at `ByteString` level. 
   
   1. Do a `prefixAndTail` of 1 element
   2. If extracted byteString prefix 
       1. is empty then stream is considered inline
       2. else merge the byteString prefix with `previousPrefix` and see if the size is still less than maxInlineSize or not
           1. If yes then recursively call `inlineAndTail` with new source and bytestring accumulated so far
           2. otherwise stream cannot be inlined and return a combined source of prefix extracted so far with tail
   
   ```scala 
     protected[database] def inlineAndTail(docStream: Source[ByteString, _],
                                           previousPrefix: ByteString = ByteString.empty)(
       implicit ec: ExecutionContext): Future[Either[ByteString, Source[ByteString, _]]] = {
       docStream.prefixAndTail(1).runWith(Sink.head).flatMap {
         case (prefix, tail) =>
           prefix.headOption.fold {
             Future.successful[Either[ByteString, Source[ByteString, _]]](Left(previousPrefix))
           } { prefix =>
             val completePrefix = previousPrefix ++ prefix
             if (completePrefix.size < maxInlineSize.toBytes) {
               inlineAndTail(tail, completePrefix)
             } else {
               Future.successful(Right(tail.prepend(Source.single(completePrefix))))
             }
           }
       }
     }
   ```
   
   With new impl the time taken to attach is similar to setup where inlining is disabled. 
   
   Some other aspects
   
   1. Re-enables the attachment inlining by setting `max-inline-size` to 16 kb (same as one before #3769)
   2. Removed the `chunk-size` config as its not needed
   3. Refactors the logic related to uploading attachment to an `AttachmentStore` to `AttachmentSupport` trait as that remains same irrespective of `ArtifactStore`
   4. Also includes a test which uploads 5MB attachment so as to check working with mid size attachments
   
   ## Related issue and scope
   <!--- Please include a link to a related issue if there is one. -->
   - [x] I opened an issue to propose and discuss this change (#3770)
   
   ## My changes affect the following components
   <!--- Select below all system components are affected by your change. -->
   <!--- Enter an `x` in all applicable boxes. -->
   - [ ] API
   - [ ] Controller
   - [ ] Message Bus (e.g., Kafka)
   - [ ] Loadbalancer
   - [ ] Invoker
   - [ ] Intrinsic actions (e.g., sequences, conductors)
   - [x] Data stores (e.g., CouchDB)
   - [ ] Tests
   - [ ] Deployment
   - [ ] CLI
   - [ ] General tooling
   - [ ] Documentation
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
   - [x] Bug fix (generally a non-breaking change which closes an issue).
   - [ ] Enhancement or new feature (adds new functionality).
   - [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
   
   ## Checklist:
   <!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
   
   - [x] I signed an [Apache CLA](https://github.com/apache/incubator-openwhisk/blob/master/CONTRIBUTING.md).
   - [x] I reviewed the [style guides](https://github.com/apache/incubator-openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
   - [x] I added tests to cover my changes.
   - [ ] My changes require further changes to the documentation.
   - [ ] I updated the documentation where necessary.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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