-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Свистухин Андрей, ИТМО DWS, Stage 5 #186
Changes from 50 commits
ca6190a
c0ffd6a
8ce7866
1eae6e8
1ec14c5
150539b
b1cb543
9de63be
10f916f
cd4d836
0ddd946
b5c5ec5
8496b42
f166923
f5a57f0
9b53f9b
c3af33d
af02092
544da9f
2902abc
c27a160
f16fb11
5591ca1
4a4b53c
07237aa
59c1274
fe68b7e
ebee01f
3f9943e
8ae8aae
2e65dfc
8c3c057
5cbc5d5
0f19d62
7c1e615
161bf45
1a60fe7
863dacc
2bf673f
f135715
8b0931e
f1664da
defaa55
bff9239
3425ed6
569a0fb
2d6c579
ae7f7ca
b503ee1
834b5ee
d8814f6
c2310c7
6dff63f
405f6aa
c10f085
0aa4829
ee6c565
6e0d9f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,89 +7,117 @@ | |
import org.slf4j.LoggerFactory; | ||
import ru.vk.itmo.ServiceConfig; | ||
|
||
import java.io.IOException; | ||
import java.net.HttpURLConnection; | ||
import java.net.URI; | ||
import java.net.http.HttpClient; | ||
import java.net.http.HttpRequest; | ||
import java.net.http.HttpResponse; | ||
import java.util.HashMap; | ||
import java.util.SortedMap; | ||
import java.util.TreeMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
import java.util.stream.Collectors; | ||
|
||
public class ProxyRequestHandler { | ||
private static final Logger log = LoggerFactory.getLogger(ProxyRequestHandler.class); | ||
|
||
private final HashMap<String, HttpClient> nodes; | ||
private final SortedMap<Integer, String> hashRing = new TreeMap<>(); | ||
private final String selfUrl; | ||
private final Map<String, HttpClient> clients; | ||
private final Map<String, Integer> urlHashes; | ||
|
||
public ProxyRequestHandler(ServiceConfig serviceConfig) { | ||
this.nodes = new HashMap<>(serviceConfig.clusterUrls().size()); | ||
this.selfUrl = serviceConfig.selfUrl(); | ||
|
||
for (int i = 0; i < serviceConfig.clusterUrls().size(); i++) { | ||
String url = serviceConfig.clusterUrls().get(i); | ||
nodes.put(url, HttpClient.newHttpClient()); | ||
int hash = Hash.murmur3(url); | ||
hashRing.put(hash, url); | ||
this.clients = new HashMap<>(); | ||
for (String url : serviceConfig.clusterUrls()) { | ||
if (!Objects.equals(url, serviceConfig.selfUrl())) { | ||
clients.put(url, HttpClient.newHttpClient()); | ||
} | ||
} | ||
this.urlHashes = serviceConfig.clusterUrls().stream() | ||
.collect(Collectors.toMap(url -> url, Hash::murmur3)); | ||
} | ||
|
||
public synchronized void close() { | ||
nodes.values().forEach(HttpClient::close); | ||
clients.values().forEach(HttpClient::close); | ||
} | ||
|
||
public Response proxyRequest(Request request) { | ||
String id = request.getParameter("id="); | ||
String nodeUrl = getNodeByKey(id); | ||
public void proxyRequests( | ||
Request request, | ||
List<String> nodeUrls, | ||
int ack, | ||
List<CompletableFuture<Response>> futures, | ||
List<Response> collectedResponses, | ||
AtomicInteger unsuccessfulResponsesCount | ||
) { | ||
AtomicInteger responsesCollected = new AtomicInteger(); | ||
|
||
HttpRequest.BodyPublisher bodyPublisher = request.getBody() == null | ||
? HttpRequest.BodyPublishers.noBody() : HttpRequest.BodyPublishers.ofByteArray(request.getBody()); | ||
for (String url : nodeUrls) { | ||
if (unsuccessfulResponsesCount.get() >= ack) { | ||
futures.add(CompletableFuture.completedFuture(null)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. тут наверное планировалось поставить return? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. если учитывать Ваш коммент выше, то получается так)) |
||
} | ||
|
||
URI uri = URI.create(nodeUrl + request.getPath() + "?id=" + id); | ||
log.debug("Proxy request to {}", uri); | ||
try { | ||
HttpResponse<byte[]> response = nodes.get(nodeUrl).send( | ||
HttpRequest.newBuilder() | ||
.uri(uri) | ||
.method(request.getMethodName(), bodyPublisher) | ||
.build(), | ||
HttpResponse.BodyHandlers.ofByteArray() | ||
); | ||
return parseResponse(response); | ||
} catch (InterruptedException ex) { | ||
log.error("Proxy request thread interrupted", ex); | ||
Thread.currentThread().interrupt(); | ||
return new Response(Response.INTERNAL_ERROR, Response.EMPTY); | ||
} catch (IOException ex) { | ||
log.error("IOException during proxy request to another node", ex); | ||
return new Response(Response.INTERNAL_ERROR, Response.EMPTY); | ||
CompletableFuture<Response> futureResponse = proxyRequest(request, url); | ||
CompletableFuture<Response> resultFuture = futureResponse.thenApply(response -> { | ||
boolean success = ServerImpl.isSuccessProcessed(response.getStatus()); | ||
if (success && responsesCollected.getAndIncrement() < ack) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. тут что-то не то, на SUCCESS ответ, который будет ack+1 мы почему-то взведем unsuccessfulResponsesCount There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Да, проглядел этот момент. Исправлю. |
||
collectedResponses.add(response); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь идет незащищенное обращение к коллекции из разных потоков, должны быть ошибки |
||
} else { | ||
unsuccessfulResponsesCount.incrementAndGet(); | ||
} | ||
return response; | ||
}); | ||
futures.add(resultFuture); | ||
} | ||
} | ||
|
||
public boolean isNeedProxy(String id) { | ||
return !getNodeByKey(id).equals(selfUrl); | ||
private CompletableFuture<Response> proxyRequest(Request request, String proxiedNodeUrl) { | ||
String id = request.getParameter("id="); | ||
byte[] body = request.getBody(); | ||
URI uri = URI.create(proxiedNodeUrl + request.getPath() + "?id=" + id); | ||
|
||
HttpRequest.Builder requestBuilder = HttpRequest.newBuilder() | ||
.uri(uri) | ||
.method( | ||
request.getMethodName(), | ||
body == null ? HttpRequest.BodyPublishers.noBody() | ||
: HttpRequest.BodyPublishers.ofByteArray(body) | ||
) | ||
.header(RequestWrapper.SELF_HEADER, "true"); | ||
|
||
CompletableFuture<HttpResponse<byte[]>> httpResponseFuture = clients.get(proxiedNodeUrl) | ||
.sendAsync(requestBuilder.build(), HttpResponse.BodyHandlers.ofByteArray()); | ||
|
||
return httpResponseFuture.thenApply(httpResponse -> { | ||
Response response = new Response(proxyResponseCode(httpResponse), httpResponse.body()); | ||
long timestamp = Long.parseLong( | ||
httpResponse.headers() | ||
.firstValue(RequestWrapper.NIO_TIMESTAMP_HEADER) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а эта бесполезная штука так и переехала дальше в этот стейдж? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Нет, она не бесполезная. В ServerImpl идет проверка, есть ли этот хедер или нет
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ага, я ошибся |
||
.orElse("0") | ||
); | ||
response.addHeader(RequestWrapper.NIO_TIMESTAMP_STRING_HEADER + timestamp); | ||
return response; | ||
}).exceptionally(ex -> { | ||
log.error("Exception during proxy request to another node", ex); | ||
return new Response(Response.INTERNAL_ERROR, Response.EMPTY); | ||
}); | ||
} | ||
|
||
private Response parseResponse(HttpResponse<byte[]> response) { | ||
String responseHttpCode = switch (response.statusCode()) { | ||
case 200 -> Response.OK; | ||
case 201 -> Response.CREATED; | ||
case 202 -> Response.ACCEPTED; | ||
case 400 -> Response.BAD_REQUEST; | ||
case 404 -> Response.NOT_FOUND; | ||
case 405 -> Response.METHOD_NOT_ALLOWED; | ||
case 503 -> Response.SERVICE_UNAVAILABLE; | ||
private String proxyResponseCode(HttpResponse<byte[]> response) { | ||
return switch (response.statusCode()) { | ||
case HttpURLConnection.HTTP_OK -> Response.OK; | ||
case HttpURLConnection.HTTP_CREATED -> Response.CREATED; | ||
case HttpURLConnection.HTTP_ACCEPTED -> Response.ACCEPTED; | ||
case HttpURLConnection.HTTP_BAD_REQUEST -> Response.BAD_REQUEST; | ||
case HttpURLConnection.HTTP_NOT_FOUND -> Response.NOT_FOUND; | ||
default -> Response.INTERNAL_ERROR; | ||
}; | ||
|
||
return new Response(responseHttpCode, response.body()); | ||
} | ||
|
||
private String getNodeByKey(String id) { | ||
int hash = Hash.murmur3(id); | ||
SortedMap<Integer, String> tailMap = hashRing.tailMap(hash); | ||
hash = tailMap.isEmpty() ? hashRing.firstKey() : tailMap.firstKey(); | ||
return hashRing.get(hash); | ||
public List<String> getNodesByHash(int numOfNodes) { | ||
return urlHashes.entrySet().stream() | ||
.sorted(Map.Entry.<String, Integer>comparingByValue().reversed()) | ||
.limit(numOfNodes) | ||
.map(Map.Entry::getKey) | ||
.collect(Collectors.toList()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
package ru.vk.itmo.test.asvistukhin; | ||
|
||
import one.nio.http.Param; | ||
import one.nio.http.Request; | ||
import one.nio.http.Response; | ||
import ru.vk.itmo.test.asvistukhin.dao.Dao; | ||
import ru.vk.itmo.test.asvistukhin.dao.TimestampEntry; | ||
|
||
import java.lang.foreign.MemorySegment; | ||
import java.lang.foreign.ValueLayout; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.List; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
|
||
public class RequestHandler { | ||
private final Dao<MemorySegment, TimestampEntry<MemorySegment>> dao; | ||
|
||
public RequestHandler(Dao<MemorySegment, TimestampEntry<MemorySegment>> dao) { | ||
this.dao = dao; | ||
} | ||
|
||
public Response handle(Request request) { | ||
String id = request.getParameter("id="); | ||
return switch (request.getMethod()) { | ||
case Request.METHOD_GET -> get(id); | ||
case Request.METHOD_PUT -> put(id, request); | ||
case Request.METHOD_DELETE -> delete(id); | ||
default -> new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
}; | ||
} | ||
|
||
public void handle( | ||
Request request, | ||
List<CompletableFuture<Response>> futures, | ||
List<Response> collectedResponses, | ||
AtomicInteger unsuccessfulResponsesCount | ||
) { | ||
futures.add(new CompletableFuture<Response>().completeAsync(() -> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а тут мы будем использовать commonPool, что плохо, лучше пользоваться своим пулом There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А нам ведь нужно создать отдельный пул под это, а не использовать пул из ServerImpl? |
||
Response response = handle(request); | ||
if (ServerImpl.isSuccessProcessed(response.getStatus())) { | ||
collectedResponses.add(response); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь идет незащищенное обращение к коллекции из разных потоков, должны быть ошибки There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а, и правда There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А, нет, я на ArrayList не менял
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. хорошо, значит я проглядел |
||
} else { | ||
unsuccessfulResponsesCount.incrementAndGet(); | ||
} | ||
|
||
return response; | ||
})); | ||
} | ||
|
||
public Response get(@Param(value = "id", required = true) String id) { | ||
if (RequestWrapper.isEmptyParam(id)) { | ||
return new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
} | ||
|
||
TimestampEntry<MemorySegment> entry = dao.get(MemorySegment.ofArray(id.getBytes(StandardCharsets.UTF_8))); | ||
|
||
Response response; | ||
long timestamp; | ||
if (entry == null || entry.value() == null) { | ||
timestamp = (entry != null ? entry.timestamp() : 0); | ||
response = new Response(Response.NOT_FOUND, Response.EMPTY); | ||
response.addHeader(RequestWrapper.TIMESTAMP_STRING_HEADER + timestamp); | ||
} else { | ||
timestamp = entry.timestamp(); | ||
response = Response.ok(entry.value().toArray(ValueLayout.JAVA_BYTE)); | ||
response.addHeader(RequestWrapper.TIMESTAMP_STRING_HEADER + timestamp); | ||
} | ||
|
||
return response; | ||
} | ||
|
||
public Response put(@Param(value = "id", required = true) String id, Request request) { | ||
if (RequestWrapper.isEmptyParam(id) || RequestWrapper.isEmptyRequest(request)) { | ||
return new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
} | ||
|
||
MemorySegment key = MemorySegment.ofArray(id.getBytes(StandardCharsets.UTF_8)); | ||
MemorySegment value = MemorySegment.ofArray(request.getBody()); | ||
dao.upsert(new TimestampEntry<>(key, value, System.currentTimeMillis())); | ||
|
||
return new Response(Response.CREATED, Response.EMPTY); | ||
} | ||
|
||
public Response delete(@Param(value = "id", required = true) String id) { | ||
if (RequestWrapper.isEmptyParam(id)) { | ||
return new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
} | ||
|
||
dao.upsert( | ||
new TimestampEntry<>( | ||
MemorySegment.ofArray(id.getBytes(StandardCharsets.UTF_8)), | ||
null, | ||
System.currentTimeMillis()) | ||
); | ||
|
||
return new Response(Response.ACCEPTED, Response.EMPTY); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package ru.vk.itmo.test.asvistukhin; | ||
|
||
import one.nio.http.Request; | ||
|
||
public class RequestWrapper { | ||
private static final String ID_PARAM = "id="; | ||
private static final String FROM_PARAM = "from="; | ||
private static final String ACK_PARAM = "ack="; | ||
public static final String SELF_HEADER = "X-Self"; | ||
public static final String NIO_TIMESTAMP_HEADER = "x-timestamp"; | ||
public static final String TIMESTAMP_STRING_HEADER = "X-Timestamp:"; | ||
public static final String NIO_TIMESTAMP_STRING_HEADER = "x-timestamp:"; | ||
|
||
public final String id; | ||
public final int from; | ||
public final int ack; | ||
|
||
public RequestWrapper(Request request, int clusterSize) throws IllegalArgumentException { | ||
String idString = request.getParameter(ID_PARAM); | ||
String fromString = request.getParameter(FROM_PARAM); | ||
String ackString = request.getParameter(ACK_PARAM); | ||
if (isEmptyParam(idString)) throw new IllegalArgumentException(); | ||
|
||
id = idString; | ||
try { | ||
from = isEmptyParam(fromString) ? clusterSize : Integer.parseInt(fromString); | ||
ack = isEmptyParam(ackString) ? (from + 1) / 2 : Integer.parseInt(ackString); | ||
} catch (NumberFormatException ex) { | ||
throw new IllegalArgumentException(ex); | ||
} | ||
|
||
if (from <= 0 || ack <= 0 || from < ack || clusterSize < from) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
|
||
public static boolean isEmptyParam(String param) { | ||
return param == null || param.isEmpty(); | ||
} | ||
|
||
public static boolean isEmptyRequest(Request request) { | ||
return request.getBody() == null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень плохой паттерн: передавать в параметрах IN и OUT переменные. Лучше стараться так организовать код, чтобы в пармтерах были только IN, а OUT было в return.
И лучше чтобы это было что-то одно
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно было эти futures возвращать return'ом и дальше записывать их в структуру которая лежит по стеку вызовов выше? Я просто подумал, что может быть из-за этого у меня и сходил с ума GC. Но кажется, проблема была всё-таки не в этом
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да, я бы сделал так, хотя понял что вы пробовали уменьшить число аллокаций