>From Wail Alkowaileet <[email protected]>:

Attention is currently required from: Hussain Towaileb, Wail Alkowaileet.
Wail Alkowaileet 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 5:

(11 comments)

Patchset:

PS5:
We need to track those TODOs ... it would be great to at least have them as 
comments (even better as JIRA issues)


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/a7bdb130_8334c8ad
PS5, Line 137: Collection<FileReference> getMatchingFiles
TODO: Remove and use list


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

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


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/e2ae8c5e_d677d734
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


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


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/c0931412_12403b75
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 ... probably you can remove this part to make 
it simpler


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/c45c3af3_e7939802
PS5, Line 88: page.remaining();
Not sure that would be accurate as two "writers" may move the position. We need 
to make sure that page.position is at 0.

You can use 'size' instead


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17453/comment/ced1d836_1e6d7397
PS5, Line 112: returnBuffer();
We probably need to move this at 'finally' scope as uploadAndWait or 
bufferedWriter.finish() could fail here and we never return the buffer


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


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/ee27888b_8d2b31ee
PS5, Line 34: LocalCacheUtil
TODO replace with a proper caching mechanism


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/cd3c4290_5a14bb0f
PS5, Line 43: Set<String> listObjects(String bucket, String path, 
FilenameFilter filter)
Comment 😊



--
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: 5
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: Hussain Towaileb <[email protected]>
Gerrit-Attention: Wail Alkowaileet
Gerrit-Comment-Date: Wed, 29 Mar 2023 18:33:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to