From Ian Maxon <[email protected]>: Attention is currently required from: Michael Blow, Peeyush Gupta.
Ian Maxon has posted comments on this change by Ian Maxon. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492?usp=email ) Change subject: [ASTERIXDB-3636][API] Migrate UDF API to domain sockets ...................................................................... Patch Set 14: (10 comments) File asterixdb/asterix-app/pom.xml: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/dd743829_4b4fffe7?usp=email : PS14, Line 873: <exclusion> : <groupId>io.netty</groupId> : <artifactId>netty-resolver-dns</artifactId> : </exclusion> : <exclusion> : <groupId>io.netty</groupId> : <artifactId>netty-resolver-dns-classes-macos</artifactId> : </exclusion> : <exclusion> : <groupId>io.netty</groupId> : <artifactId>netty-resolver-dns-native-macos</artifactId> : </exclusion> > i think these three shouldn't be needed since they are already under > dependency management, but it c […] lemme remove them then, i was trying to be maximal about it to banish any native classes File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/task/RetrieveLibrariesTask.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/e7b98403_d2fd3c8f?usp=email : PS14, Line 52: transient > I don't think this can have any impact, since this is a final field. Done File asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/f4ef3452_71859f3f?usp=email : PS14, Line 278: Pair<Map<String, String>, Map<String, String>> auth = : BasicAuthServlet.generateSysAuthHeader(apiServer.ctx()); : apiServer.addServlet((new BasicAuthServlet(apiServer.ctx(), : new NCUdfApiServlet(apiServer.ctx(), new String[] { UDF }, getApplicationContext(), : apiServer.getScheme(), externalProperties.getNcApiPort()), : auth.getFirst(), auth.getSecond()))); : apiServer.addServlet(new BasicAuthServlet(apiServer.ctx(), : new NCUdfRecoveryServlet(apiServer.ctx(), new String[] { UDF_RECOVERY }, getApplicationContext()), : auth.getFirst(), auth.getSecond())); : > are we going to continue to support the old UDFs? for the time being in asterix yeah, that was my thought anyway. probably not forever File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/e38ea2e3_11c1383a?usp=email : PS14, Line 111: temporary directory/asterixdb_udf/$NODE_ID_udf.sock > can we make this `<tmpdir>/asterixdb_udf/${NODE_ID}_udf. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/dd325677_c112ba60?usp=email : PS14, Line 254: Defaults to default.dir/udf.sock > i don't think we usually include the default desc here, since it already is > picked up as a separate […] ah right, forgot about that. fixed File hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/ac8dff1f_31e7c983?usp=email : PS14, Line 129: private String getPort(SocketAddress s) { : if (s instanceof InetSocketAddress) { : return NetworkUtil.toHostPort((InetSocketAddress) channel.localAddress()); : } : return "0"; : } : > what is this supposed to return? I don't quite get how "0" is a host:port it isn't, i could return null. domain sockets don't have ports like TCP sockets do. File hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServer.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/99b39e97_8c1324c0?usp=email : PS14, Line 139: return addr.toString(); > what can `addr` aside from a `InetSocketAddress` or a > `UnixDomainSocketAddress`? some of the netty native transports expose a different implementation (e.g. DomainSocketAddress). in principle it should only be these two implementations that this method should recieve https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/41374555_51679686?usp=email : PS14, Line 160: int[] ports = addresses.stream().filter(s -> s instanceof InetSocketAddress) : .mapToInt(s -> ((InetSocketAddress) s).getPort()).distinct().toArray(); > ```suggestion […] that is neat, i didn't know you could refer to a method like that. applied the same pattern to 164/5 as well https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/888268cb_d2c90723?usp=email : PS14, Line 418: deleteDomainSockFiles(); > what happens if we don't clean these up? should we do it in a finally? if you don't clean it up and attempt to rebind the socket on the same file, it will fail like when you attempt to bind an address that another program is already holding the one you want. it would be good to put it in a finally but i couldn't find a nice one to put this in. let me try and find one. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492/comment/0ffd67de_e0575696?usp=email : PS14, Line 479: so we need to clean them up on shutdown > what happens if we don't clean these up, e.g. […] see comment on 418 -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20492?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I5f8ac2170fd6b2beef14d99c38b9141af0f12ba3 Gerrit-Change-Number: 20492 Gerrit-PatchSet: 14 Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-CC: Hussain Towaileb <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Attention: Michael Blow <[email protected]> Gerrit-Comment-Date: Wed, 18 Feb 2026 00:09:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Blow <[email protected]>
