Return-Path: X-Original-To: apmail-asterixdb-notifications-archive@minotaur.apache.org Delivered-To: apmail-asterixdb-notifications-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id CC2E318C4F for ; Mon, 30 Nov 2015 22:46:52 +0000 (UTC) Received: (qmail 43596 invoked by uid 500); 30 Nov 2015 22:46:52 -0000 Delivered-To: apmail-asterixdb-notifications-archive@asterixdb.apache.org Received: (qmail 43564 invoked by uid 500); 30 Nov 2015 22:46:52 -0000 Mailing-List: contact notifications-help@asterixdb.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@asterixdb.incubator.apache.org Delivered-To: mailing list notifications@asterixdb.incubator.apache.org Received: (qmail 43550 invoked by uid 99); 30 Nov 2015 22:46:52 -0000 Received: from Unknown (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 30 Nov 2015 22:46:52 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 430D7C1489 for ; Mon, 30 Nov 2015 22:46:47 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.92 X-Spam-Level: X-Spam-Status: No, score=0.92 tagged_above=-999 required=6.31 tests=[SPF_FAIL=0.919, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id 0BLhn7yKmn1C for ; Mon, 30 Nov 2015 22:46:40 +0000 (UTC) Received: from unhygienix.ics.uci.edu (unhygienix.ics.uci.edu [128.195.14.130]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with ESMTP id 8CB75429A6 for ; Mon, 30 Nov 2015 22:46:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by unhygienix.ics.uci.edu (Postfix) with ESMTP id C0E6B240016; Mon, 30 Nov 2015 14:43:22 -0800 (PST) Date: Mon, 30 Nov 2015 14:43:22 -0800 From: "Till Westmann (Code Review)" To: Murtadha Hubail CC: Jenkins , abdullah alamoudi , Ian Maxon Reply-To: tillw@apache.org X-Gerrit-MessageType: comment Subject: Change in hyracks[master]: Add Global Resource Id Factory X-Gerrit-Change-Id: Ib9f49234eebe912c48e7f71980433a9b42595741 X-Gerrit-ChangeURL: X-Gerrit-Commit: bc5bccf135a38c6944367f1308a466a1e6962ebf In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/2.8.4 Message-Id: <20151130224322.C0E6B240016@unhygienix.ics.uci.edu> Till Westmann has posted comments on this change. Change subject: Add Global Resource Id Factory ...................................................................... Patch Set 3: (8 comments) https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/ICCApplicationEntryPoint.java File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/ICCApplicationEntryPoint.java: Line 28: public boolean isReadyToProcessUnqiueIdRequests(); Does this need to be on the ApplicationEntryPoint? Would the ApplicationContext be more appropriate? https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/INCApplicationEntryPoint.java File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/INCApplicationEntryPoint.java: Line 32: public long getNCMaxUniqueId() throws Exception; This doesn't seem to fit in very well with the lifecycle-oriented methods on this interface? Is there a better place? https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksRootContext.java File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksRootContext.java: Line 32: Does this need to be part of the RootContext? That seems very high up in the hierarchy. https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/base/IClusterController.java File hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/base/IClusterController.java: Line 75: public void getUnqiueId(InetSocketAddress ncAddress, String nodeId) throws Exception; I think that we shouldn't need the ncAddress here? Isn't the nodeId enough to identify the NC (and thus the address)? https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/CCNCFunctions.java File hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/CCNCFunctions.java: Line 1265: private final InetSocketAddress ncAddress; Again, I think the we can do without this address. https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/ClusterControllerRemoteProxy.java File hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/ClusterControllerRemoteProxy.java: Line 158: public void getUnqiueId(InetSocketAddress ncAddress, String nodeId) throws Exception { Again, I think the we can do without this address. https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NodeControllerService.java File hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NodeControllerService.java: Line 165: private final LinkedBlockingQueue uniqueIdResponseQ; Why do we need a new queue here? https://asterix-gerrit.ics.uci.edu/#/c/485/3/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/GlobalResourceIdFactory.java File hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/GlobalResourceIdFactory.java: Line 28: private final IHyracksRootContext rootCtx; Again, this might not need to be the root context. -- To view, visit https://asterix-gerrit.ics.uci.edu/485 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9f49234eebe912c48e7f71980433a9b42595741 Gerrit-PatchSet: 3 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail Gerrit-Reviewer: Ian Maxon Gerrit-Reviewer: Jenkins Gerrit-Reviewer: Murtadha Hubail Gerrit-Reviewer: Till Westmann Gerrit-Reviewer: abdullah alamoudi Gerrit-HasComments: Yes