BsoBird commented on code in PR #10688: URL: https://github.com/apache/iceberg/pull/10688#discussion_r1675655377
########## core/src/main/java/org/apache/iceberg/lock/ServerSideHttpLockManager.java: ########## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.lock; + +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Map; +import org.apache.http.HttpResponse; +import org.apache.http.StatusLine; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.protocol.RequestAcceptEncoding; +import org.apache.http.client.protocol.ResponseContentEncoding; +import org.apache.http.client.utils.URIBuilder; +import org.apache.http.impl.client.DefaultHttpClient; +import org.apache.http.params.BasicHttpParams; +import org.apache.http.params.CoreConnectionPNames; +import org.apache.http.params.HttpParams; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.util.LockManagers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * The user is required to provide an API interface with distributed locking functionality as + * agreed. + */ +public class ServerSideHttpLockManager extends LockManagers.BaseLockManager { + private static final Logger LOG = LoggerFactory.getLogger(ServerSideHttpLockManager.class); + private String httpUrl = null; + private static final String OPERATOR = "operator"; + private static final String LOCK = "lock"; + private static final String UNLOCK = "unlock"; + private static final String ENTITY_ID = "entityId"; + private static final String OWNER_ID = "ownerId"; + private static final int REQUEST_SUCCESS = 200; + public static final String REQUEST_URL = "lock.http.conf.request.url"; + private HttpClient httpClient = null; + + public ServerSideHttpLockManager() {} + + public ServerSideHttpLockManager(String requestUrl) { + initialize(ImmutableMap.of(REQUEST_URL, requestUrl)); + } + + @Override + public boolean acquire(String entityId, String ownerId) { + return process(entityId, ownerId, LOCK); + } + + @Override + public void initialize(Map<String, String> properties) { + super.initialize(properties); + init(properties.getOrDefault(REQUEST_URL, null)); + } + + @Override + public void close() throws Exception { + super.close(); + if (httpClient != null) { + httpClient.getConnectionManager().shutdown(); + } + } + + private synchronized void init(String requestUrl) { + if (requestUrl == null) { + String msg = String.format("[%s] must be set.", REQUEST_URL); + throw new IllegalArgumentException(msg); + } + if (this.httpUrl == null) { + this.httpUrl = requestUrl; + } + if (this.httpClient == null) { + DefaultHttpClient defaultHttpClient = new DefaultHttpClient(); + defaultHttpClient.addRequestInterceptor(new RequestAcceptEncoding()); + defaultHttpClient.addResponseInterceptor(new ResponseContentEncoding()); + defaultHttpClient + .getParams() + .setParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, acquireTimeoutMs()); + defaultHttpClient + .getParams() + .setParameter(CoreConnectionPNames.SO_TIMEOUT, heartbeatTimeoutMs()); + this.httpClient = new DefaultHttpClient(); + } + } + + private String encode(String entity) { + if (entity == null) { + return null; + } + return new String( + Base64.getUrlEncoder().encode(entity.getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); + } + + private boolean process(String entityId, String ownerId, String operator) { + try { + HttpGet lockRequest = new HttpGet(); + lockRequest.addHeader("Content-Type", "application/json"); + HttpParams requestParams = new BasicHttpParams(); + requestParams.setParameter(OWNER_ID, encode(ownerId)); + requestParams.setParameter(ENTITY_ID, encode(entityId)); + requestParams.setParameter(OPERATOR, operator); + String requestUrl = + new URIBuilder(httpUrl) + .setParameter(OWNER_ID, encode(ownerId)) + .setParameter(ENTITY_ID, encode(entityId)) + .setParameter(OPERATOR, operator) + .build() + .toString(); + lockRequest.setParams(requestParams); + lockRequest.setURI(new URI(requestUrl)); + HttpResponse response = httpClient.execute(lockRequest); + StatusLine stateLine = response.getStatusLine(); + int statsCode = stateLine.getStatusCode(); + lockRequest.abort(); + return REQUEST_SUCCESS == statsCode; Review Comment: To be honest, I don't think we need to be concerned with the exact cause of the failure.Also, since we don't know what data the user intends to return, parsing the data returned by the user introduces a significant amount of logical complexity. Instead of doing this, it's easier to just detect the HTTP status code. It's not difficult for users to implement the rest-api service in the way we want. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org