>From Hussain Towaileb <[email protected]>:

Attention is currently required from: Wail Alkowaileet.
Hussain Towaileb has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453 )

Change subject: [ASTERIXDB-3158][HYR]: Make IOManager support writing to cloud 
storage
......................................................................


Patch Set 6: Code-Review+1

(10 comments)

File 
hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/io/IIOManager.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/44625929_790941b2
PS5, Line 137: Collection<FileReference> getMatchingFiles
> TODO: Remove and use list
Ack


File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/pom.xml:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/de8a051b_c9ac44e2
PS5, Line 32: awsjavasdk.version>2.17.218</awsjavasdk.version
> Move to hyracks main pom?
Done


File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/CloudIOManager.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/e8eb87f0_ac0bf585
PS5, Line 109:                 LOGGER.info("Downloading {} from S3..", 
fileRef.getRelativePath());
             :                 LocalCacheUtil.download(cloudClient, this, 
fHandle, rwMode, syncMode, writeBuffer);
             :                 super.close(fHandle);
             :                 LOGGER.info("Finished downloading {} from S3..", 
fileRef.getRelativePath());
> TODO: We need a proper caching mechanism
I have an idea for this, let's discuss it once we have the chance to sync.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/a0352b99_c4189655
PS5, Line 235: return cloudClient.readAllBytes(bucket, 
fileRef.getRelativePath());
> TODO: We need to download this too
Ack


File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/CloudResettableInputStream.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/3bc836d1_5ba0fa28
PS5, Line 80:       // enough to write all
            :         if (writeBuffer.remaining() > size) {
            :             writeBuffer.put(page.array(), 0, size);
            :             return size;
            :         }
> The logic below covers this case ... […]
Forgot to remove this part, good catch 👍
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/33a14f5b_b5f681e7
PS5, Line 88: page.remaining();
> Not sure that would be accurate as two "writers" may move the position. […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/76f55610_e95f0998
PS5, Line 112: returnBuffer();
> We probably need to move this at 'finally' scope as uploadAndWait or 
> bufferedWriter. […]
Done


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/447a83fd_55dcbaac
PS5, Line 117: returnBuffer()
> Same as above, add in the finally scope
Done


File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/LocalCacheUtil.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/ee5a93f6_90437cad
PS5, Line 34: LocalCacheUtil
> TODO replace with a proper caching mechanism
Ack


File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/cloud/clients/ICloudClient.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/d319835b_75849f82
PS5, Line 43: Set<String> listObjects(String bucket, String path, 
FilenameFilter filter)
> Comment 😊
Done



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453
To unsubscribe, or for help writing mail filters, visit 
https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I362a3cbfcd3fa99f321467cb72d74d388fcdee2b
Gerrit-Change-Number: 17453
Gerrit-PatchSet: 6
Gerrit-Owner: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Wail Alkowaileet
Gerrit-Reviewer: Wail Alkowaileet <[email protected]>
Gerrit-Attention: Wail Alkowaileet <[email protected]>
Gerrit-Attention: Wail Alkowaileet
Gerrit-Comment-Date: Thu, 30 Mar 2023 00:14:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Wail Alkowaileet <[email protected]>
Gerrit-MessageType: comment

Reply via email to