You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by blrunner <gi...@git.apache.org> on 2015/07/08 05:10:23 UTC
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
GitHub user blrunner opened a pull request:
https://github.com/apache/tajo/pull/625
TAJO-1677: Remove unnecessary messages for the Travis CI build
Enable -B (batch) mode on the build. This will make the logs shorter since it avoids the dependency download progress logging.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/blrunner/tajo TAJO-1677
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/625.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #625
----
commit ebdfab1dfac506eef00baf1694314b1bef65082c
Author: JaeHwa Jung <bl...@apache.org>
Date: 2015-07-08T03:09:08Z
Enable batch mode in maven.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-120861596
Hi @hyunsik
Thank you for your detailed review.
I've updated the patch using your comments.
For your information, I added -q parameter instead of -B parameter. In some unexplained way, -B parameter didn't work as expect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-121134338
Thanks @hyunsik
I recovered codes related to coding format.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner closed the pull request at:
https://github.com/apache/tajo/pull/625
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-121146712
+1 LGTM!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437203
--- Diff: tajo-core/src/test/java/org/apache/tajo/engine/planner/physical/TestExternalSortExec.java ---
@@ -98,8 +102,10 @@ public void setUp() throws Exception {
appender.flush();
appender.close();
- System.out.println(appender.getStats().getNumRows() + " rows (" + (appender.getStats().getNumBytes() / 1048576) +
+ if (LOG.isDebugEnabled()) {
--- End diff --
This if-condition branch seems to be not necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-119872485
I've implemented this patch as follows.
- Apply -q parameter for maven : Only show errors.
- Apply -ff parameter for maven : Stop at first failure in reactorized builds
- Apply surefire.useFile=false for maven: This causes surefire to print test failures to standard out.
- Clean up TestSystemMetrics log messages.
- Clean up TajoMaster log messages.
- Fix TestBSTIndexExec:testEqual error.
- Remove RestServer warning log messages.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437250
--- Diff: tajo-core/src/test/java/org/apache/tajo/worker/TestNodeResourceManager.java ---
@@ -253,7 +256,9 @@ public void run() {
fail(e.getMessage());
}
}
- System.out.println(Thread.currentThread().getName() + " complete requests: " + complete);
+ if (LOG.isDebugEnabled()) {
--- End diff --
This if-condition branch seems to be not necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34472909
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java ---
@@ -236,7 +236,7 @@ private void initSystemMetrics() {
private void initResourceManager() throws Exception {
Class<WorkerResourceManager> resourceManagerClass = (Class<WorkerResourceManager>)
- systemConf.getClass(ConfVars.RESOURCE_MANAGER_CLASS.varname, TajoWorkerResourceManager.class);
+ systemConf.getClass(ConfVars.RESOURCE_MANAGER_CLASS.varname, TajoWorkerResourceManager.class);
--- End diff --
You need to recover meaningless changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-120835927
The first description mentioned that -B will be added to maven option. Was the plan changed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34472861
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java ---
@@ -202,7 +202,7 @@ public void serviceInit(Configuration _conf) throws Exception {
tajoMasterService = new QueryCoordinatorService(context);
addIfService(tajoMasterService);
-
+
--- End diff --
You need to recover meaningless changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437214
--- Diff: tajo-core/src/test/java/org/apache/tajo/engine/planner/physical/TestProgressExternalSortExec.java ---
@@ -102,7 +106,9 @@ public void setUp() throws Exception {
appender.flush();
appender.close();
- System.out.println(appender.getStats().getNumRows() + " rows (" + appender.getStats().getNumBytes() + " Bytes)");
+ if (LOG.isDebugEnabled()) {
--- End diff --
This if-condition branch seems to be not necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-120861128
Please see the following volumes. quite.txt contains all messages from '-q' option. For me, `-q` seems to be not effective.
```
hyunsik@Hyunsiks-MacBook-Pro:~$ ls -l quite.txt normal.txt
-rw-r--r-- 1 hyunsik staff 75536879 Jul 13 16:41 normal.txt
-rw-r--r-- 1 hyunsik staff 75490962 Jul 13 17:14 quite.txt
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437155
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java ---
@@ -280,19 +280,21 @@ private void checkAndInitializeSystemDirectories() throws IOException {
// Get Warehouse dir
this.wareHousePath = TajoConf.getWarehouseDir(systemConf);
- LOG.info("Tajo Warehouse dir: " + wareHousePath);
// Check and Create Warehouse dir
if (!defaultFS.exists(wareHousePath)) {
defaultFS.mkdirs(wareHousePath, new FsPermission(WAREHOUSE_DIR_PERMISSION));
LOG.info("Warehouse dir '" + wareHousePath + "' is created");
+ } else {
+ LOG.info("Tajo Warehouse dir: " + wareHousePath);
}
Path stagingPath = TajoConf.getDefaultRootStagingDir(systemConf);
- LOG.info("Staging dir: " + wareHousePath);
if (!defaultFS.exists(stagingPath)) {
defaultFS.mkdirs(stagingPath, new FsPermission(STAGING_ROOTDIR_PERMISSION));
LOG.info("Staging dir '" + stagingPath + "' is created");
+ } else {
+ LOG.info("Staging dir: " + wareHousePath);
--- End diff --
It should be recover due to the same reason as I commented above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-120962644
As I mentioned in some comments, most of changes are about coding format. You need to recover them. Coding format should be done in another issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437152
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java ---
@@ -280,19 +280,21 @@ private void checkAndInitializeSystemDirectories() throws IOException {
// Get Warehouse dir
this.wareHousePath = TajoConf.getWarehouseDir(systemConf);
- LOG.info("Tajo Warehouse dir: " + wareHousePath);
// Check and Create Warehouse dir
if (!defaultFS.exists(wareHousePath)) {
defaultFS.mkdirs(wareHousePath, new FsPermission(WAREHOUSE_DIR_PERMISSION));
LOG.info("Warehouse dir '" + wareHousePath + "' is created");
+ } else {
+ LOG.info("Tajo Warehouse dir: " + wareHousePath);
--- End diff --
It should be recover due to the same reason as I commented above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/625#issuecomment-120866803
I think that the parameter on your desktop seemed like ineffective, because tajo dependencies already had been installed on your desktop. If you see build progress of this PR, you would see that there is no dependency download progress.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437253
--- Diff: tajo-core/src/test/java/org/apache/tajo/worker/TestNodeResourceManager.java ---
@@ -264,8 +269,10 @@ public void run() {
future.get();
}
- System.out.println(parallelCount + " Thread, completed requests: " + totalComplete.get() + ", canceled requests:"
+ if (LOG.isDebugEnabled()) {
--- End diff --
This if-condition branch seems to be not necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1677: Remove unnecessary messages for the ...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/625#discussion_r34437106
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java ---
@@ -255,15 +255,15 @@ private void initWebServer() throws Exception {
private void checkAndInitializeSystemDirectories() throws IOException {
// Get Tajo root dir
this.tajoRootPath = TajoConf.getTajoRootDir(systemConf);
- LOG.info("Tajo Root Directory: " + tajoRootPath);
// Check and Create Tajo root dir
this.defaultFS = tajoRootPath.getFileSystem(systemConf);
systemConf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, defaultFS.getUri().toString());
- LOG.info("FileSystem (" + this.defaultFS.getUri() + ") is initialized.");
if (!defaultFS.exists(tajoRootPath)) {
defaultFS.mkdirs(tajoRootPath, new FsPermission(TAJO_ROOT_DIR_PERMISSION));
LOG.info("Tajo Root Directory '" + tajoRootPath + "' is created.");
+ } else {
+ LOG.info("FileSystem (" + this.defaultFS.getUri() + ") is initialized.");
--- End diff --
This change seems to be different to the original intension. This log appears to show which HDFS namenode is used in this TajoMaster.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---