asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Maxon (Code Review)" <do-not-re...@asterix-gerrit.ics.uci.edu>
Subject Change in asterixdb[master]: Periodically saving JSON data from admin console API to a te...
Date Mon, 17 Aug 2015 19:30:45 GMT
Ian Maxon has posted comments on this change.

Change subject: Periodically saving JSON data from admin console API to a temporary dataset.
......................................................................


Patch Set 7:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/350/7/asterix-app/src/main/java/edu/uci/ics/asterix/hyracks/bootstrap/AsterixGlobalRecoveryManager.java
File asterix-app/src/main/java/edu/uci/ics/asterix/hyracks/bootstrap/AsterixGlobalRecoveryManager.java:

Line 192: 
This doesn't actually do what the comment implies. You're just checking if the thread is alive,
that doesn't mean the cluster is actually up. What if, this thead was blocked, and the recovery
thread already started and finished before that check happened? You need to check INSTANCE.getState()
to figure out what the cluster's state is. Not busy-waiting to accomplish that would also
be good.


https://asterix-gerrit.ics.uci.edu/#/c/350/7/asterix-app/src/main/java/edu/uci/ics/asterix/hyracks/bootstrap/GatherJSONData.java
File asterix-app/src/main/java/edu/uci/ics/asterix/hyracks/bootstrap/GatherJSONData.java:

Line 101:                 try {
This interval should probably be a configuration parameter.


Line 111:         final String url = "http://localhost:19002/ddl";
It can't be assumed that the CC is listening on a particular host or port. This should be
constructed from the configuration parameters, which should already be accessible from here
without too much work, since this is running on the CC.


Line 123:         HttpHost target = new HttpHost("localhost", 8888, "http");
Same goes for this host and port. The default is 8888 but that doesn't mean it always will
be.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/350
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cb186abda6a14d9eb866259d459ce5b5e855be8
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Pritom Ahmed <pritom.11@gmail.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-HasComments: Yes

Mime
View raw message