Michael Blow has posted comments on this change. Change subject: Replace Servlets with Netty Based HTTP Servers ......................................................................
Patch Set 1: (5 comments) (partial review) https://asterix-gerrit.ics.uci.edu/#/c/1429/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiLet.java: Line 109: hds = (IHyracksDataset) map.get(HYRACKS_DATASET_ATTR); This is not thread safe, this can return a partially initialized IHyracksDataset as coded here. https://asterix-gerrit.ics.uci.edu/#/c/1429/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterApiLet.java: Line 128: requestURL.append(request.getHeader(HttpHeaderNames.HOST.toString())); Update getHeader(String) -> getHeader(CharSequence)? https://asterix-gerrit.ics.uci.edu/#/c/1429/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ILet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ILet.java: Line 23: public interface ILet { Why Let? Seems a little too nondescript. Can we stick with Servlet, or use Httplet, Weblet, something...? https://asterix-gerrit.ics.uci.edu/#/c/1429/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ILetResponse.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ILetResponse.java: Line 83: response.addHeader(HttpHeaderNames.CONTENT_TYPE, type + "; charset=" + charset); It seems this won't do the right thing if setCharacterEndoding/setContentType() is called twice, as addHeader seems additive... https://asterix-gerrit.ics.uci.edu/#/c/1429/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryStatusApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryStatusApiLet.java: Line 60: hds = (IHyracksDataset) map.get(HYRACKS_DATASET_ATTR); This is not thread safe, this can return a partially initialized IHyracksDataset as coded here. -- To view, visit https://asterix-gerrit.ics.uci.edu/1429 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d552d4eb8c868535ca4c41cbcf7e352217b18ae Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-HasComments: Yes
