Yingyi Bu has posted comments on this change. Change subject: This change includes the following things: 1. Allow pregelix statement to cross two different dataverses. 2. Change TestsUtils to TestsExecutor which is not based on static methods so that Pregelix can reuse its functionalities. 3. Adds ioDeviceId into th ......................................................................
Patch Set 3: (7 comments) https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/api/common/AsterixHyracksIntegrationUtil.java File asterix-app/src/main/java/edu/uci/ics/asterix/api/common/AsterixHyracksIntegrationUtil.java: Line 63: File dir = new File(System.getProperty("java.io.tmpdir") + File.separator + "nc1"); > This "java.io.tmpdir" is used multiple times, can we make is as a static fi Done Line 65: FileUtils.forceDelete(dir); > What kind of problem will appear if we didn't delete? It is to delete test storage files (LSM etc.) in case there are aborted tests before. https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/ConnectorAPIServlet.java File asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/ConnectorAPIServlet.java: Line 111: // Flush the cached contents of the dataset to file system. > Why do we need to flush at this time? Especially in the "doGet" method, do External runtimes like Pregelix use this GET method to get the description of the dataset and read the dataset by themselves. In case part of the requested dataset is not flushed, e.g., having in-memory LSM components, we need to flush (not force) it to make sure the data goes to the OS file system so that other runtimes can see the data. For read, we need this "flush" to get the up-to-date data. https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java File asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java: Line 2829: if (resultState != 1) { > why do we take the 1 as the succeed? Should it be 0 by the bash convention? Done Line 2926: pr.waitFor(); > Should we catch the `pr.exitValue()` as well? In case some other system err Done Line 2972: if (s.equals("-cust-prop")) { > just curiously, what's this "-cust-prop" meaning? it is an option in Pregelix--customized properties. A user can put a sequence of comma-separated key=value pairs for that parameter. Those properties will be written to the job's configuration XML files as key-value pairs. https://asterix-gerrit.ics.uci.edu/#/c/325/3/asterix-app/src/test/java/edu/uci/ics/asterix/aql/translator/AqlTranslatorTest.java File asterix-app/src/test/java/edu/uci/ics/asterix/aql/translator/AqlTranslatorTest.java: Line 78: .asList(new String[] { "bin/pregelix examples/pregelix-example-0.2.13-jar-with-dependencies.jar " > Should we make this 0.2.13 be pluggable in order to not break the test once It could be any xyz.jar at line 68. The expected result only depends on line 68 but has nothing to do with the current Pregelix version. I removed the version number anyway. -- To view, visit https://asterix-gerrit.ics.uci.edu/325 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d167b64bf9ec754182b5b2fe44dfc7e5908c686 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
