Don’t persist ephemeral messages; clear ephemeral field when sending to clients
This commit is contained in:
parent
54fe3b9a43
commit
a7443a9ece
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright 2013-2020 Signal Messenger, LLC
|
||||
* Copyright 2013-2021 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
|
@ -39,7 +39,6 @@ public class MessagePersister implements Managed {
|
|||
private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
|
||||
private final Timer getQueuesTimer = metricRegistry.timer(name(MessagePersister.class, "getQueues"));
|
||||
private final Timer persistQueueTimer = metricRegistry.timer(name(MessagePersister.class, "persistQueue"));
|
||||
private final Meter persistMessageMeter = metricRegistry.meter(name(MessagePersister.class, "persistMessage"));
|
||||
private final Meter persistQueueExceptionMeter = metricRegistry.meter(name(MessagePersister.class, "persistQueueException"));
|
||||
private final Histogram queueCountHistogram = metricRegistry.histogram(name(MessagePersister.class, "queueCount"));
|
||||
private final Histogram queueSizeHistogram = metricRegistry.histogram(name(MessagePersister.class, "queueSize"));
|
||||
|
@ -161,7 +160,7 @@ public class MessagePersister implements Managed {
|
|||
messagesManager.persistMessages(accountUuid, deviceId, messages);
|
||||
messageCount += messages.size();
|
||||
|
||||
persistMessageMeter.mark(messages.size());
|
||||
|
||||
} while (!messages.isEmpty());
|
||||
|
||||
queueSizeHistogram.update(messageCount);
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright 2013-2020 Signal Messenger, LLC
|
||||
* Copyright 2013-2021 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
|
@ -135,7 +135,7 @@ public class MessagesCache extends RedisClusterPubSubAdapter<String, String> imp
|
|||
|
||||
public long insert(final UUID guid, final UUID destinationUuid, final long destinationDevice, final MessageProtos.Envelope message) {
|
||||
final MessageProtos.Envelope messageWithGuid = message.toBuilder().setServerGuid(guid.toString()).build();
|
||||
return (long)insertTimer.record(() ->
|
||||
return (long) insertTimer.record(() ->
|
||||
insertScript.executeBinary(List.of(getMessageQueueKey(destinationUuid, destinationDevice),
|
||||
getMessageQueueMetadataKey(destinationUuid, destinationDevice),
|
||||
getQueueIndexKey(destinationUuid, destinationDevice)),
|
||||
|
@ -144,28 +144,42 @@ public class MessagesCache extends RedisClusterPubSubAdapter<String, String> imp
|
|||
guid.toString().getBytes(StandardCharsets.UTF_8))));
|
||||
}
|
||||
|
||||
public void insertEphemeral(final UUID destinationUuid, final long destinationDevice, final MessageProtos.Envelope message) {
|
||||
public void insertEphemeral(final UUID destinationUuid, final long destinationDevice,
|
||||
final MessageProtos.Envelope message) {
|
||||
|
||||
final MessageProtos.Envelope messageWithGuid;
|
||||
|
||||
if (!message.hasServerGuid()) {
|
||||
messageWithGuid = message.toBuilder().setServerGuid(UUID.randomUUID().toString()).build();
|
||||
} else {
|
||||
messageWithGuid = message;
|
||||
}
|
||||
|
||||
insertEphemeralTimer.record(() -> {
|
||||
final byte[] ephemeralQueueKey = getEphemeralMessageQueueKey(destinationUuid, destinationDevice);
|
||||
|
||||
insertCluster.useBinaryCluster(connection -> {
|
||||
connection.sync().rpush(ephemeralQueueKey, message.toByteArray());
|
||||
connection.sync().rpush(ephemeralQueueKey, messageWithGuid.toByteArray());
|
||||
connection.sync().expire(ephemeralQueueKey, MAX_EPHEMERAL_MESSAGE_DELAY.toSeconds());
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
public Optional<OutgoingMessageEntity> remove(final UUID destinationUuid, final long destinationDevice, final UUID messageGuid) {
|
||||
public Optional<OutgoingMessageEntity> remove(final UUID destinationUuid, final long destinationDevice,
|
||||
final UUID messageGuid) {
|
||||
return remove(destinationUuid, destinationDevice, List.of(messageGuid)).stream().findFirst();
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
public List<OutgoingMessageEntity> remove(final UUID destinationUuid, final long destinationDevice, final List<UUID> messageGuids) {
|
||||
final List<byte[]> serialized = (List<byte[]>)Metrics.timer(REMOVE_TIMER_NAME, REMOVE_METHOD_TAG, REMOVE_METHOD_UUID).record(() ->
|
||||
public List<OutgoingMessageEntity> remove(final UUID destinationUuid, final long destinationDevice,
|
||||
final List<UUID> messageGuids) {
|
||||
final List<byte[]> serialized = (List<byte[]>) Metrics.timer(REMOVE_TIMER_NAME, REMOVE_METHOD_TAG,
|
||||
REMOVE_METHOD_UUID).record(() ->
|
||||
removeByGuidScript.executeBinary(List.of(getMessageQueueKey(destinationUuid, destinationDevice),
|
||||
getMessageQueueMetadataKey(destinationUuid, destinationDevice),
|
||||
getQueueIndexKey(destinationUuid, destinationDevice)),
|
||||
messageGuids.stream().map(guid -> guid.toString().getBytes(StandardCharsets.UTF_8)).collect(Collectors.toList())));
|
||||
messageGuids.stream().map(guid -> guid.toString().getBytes(StandardCharsets.UTF_8))
|
||||
.collect(Collectors.toList())));
|
||||
|
||||
final List<OutgoingMessageEntity> removedMessages = new ArrayList<>(serialized.size());
|
||||
|
||||
|
@ -233,18 +247,21 @@ public class MessagesCache extends RedisClusterPubSubAdapter<String, String> imp
|
|||
});
|
||||
}
|
||||
|
||||
public Optional<MessageProtos.Envelope> takeEphemeralMessage(final UUID destinationUuid, final long destinationDevice) {
|
||||
public Optional<MessageProtos.Envelope> takeEphemeralMessage(final UUID destinationUuid,
|
||||
final long destinationDevice) {
|
||||
return takeEphemeralMessage(destinationUuid, destinationDevice, System.currentTimeMillis());
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
Optional<MessageProtos.Envelope> takeEphemeralMessage(final UUID destinationUuid, final long destinationDevice, final long currentTimeMillis) {
|
||||
Optional<MessageProtos.Envelope> takeEphemeralMessage(final UUID destinationUuid, final long destinationDevice,
|
||||
final long currentTimeMillis) {
|
||||
final long earliestAllowableTimestamp = currentTimeMillis - MAX_EPHEMERAL_MESSAGE_DELAY.toMillis();
|
||||
|
||||
return takeEphemeralMessageTimer.record(() -> readDeleteCluster.withBinaryCluster(connection -> {
|
||||
byte[] messageBytes;
|
||||
|
||||
while ((messageBytes = connection.sync().lpop(getEphemeralMessageQueueKey(destinationUuid, destinationDevice))) != null) {
|
||||
while ((messageBytes = connection.sync().lpop(getEphemeralMessageQueueKey(destinationUuid, destinationDevice)))
|
||||
!= null) {
|
||||
try {
|
||||
final MessageProtos.Envelope message = MessageProtos.Envelope.parseFrom(messageBytes);
|
||||
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright 2013-2020 Signal Messenger, LLC
|
||||
* Copyright 2013-2021 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
package org.whispersystems.textsecuregcm.storage;
|
||||
|
@ -26,8 +26,12 @@ public class MessagesManager {
|
|||
private static final int RESULT_SET_CHUNK_SIZE = 100;
|
||||
|
||||
private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
|
||||
private static final Meter cacheHitByGuidMeter = metricRegistry.meter(name(MessagesManager.class, "cacheHitByGuid" ));
|
||||
private static final Meter cacheMissByGuidMeter = metricRegistry.meter(name(MessagesManager.class, "cacheMissByGuid"));
|
||||
private static final Meter cacheHitByGuidMeter = metricRegistry.meter(name(MessagesManager.class, "cacheHitByGuid"));
|
||||
private static final Meter cacheMissByGuidMeter = metricRegistry.meter(
|
||||
name(MessagesManager.class, "cacheMissByGuid"));
|
||||
|
||||
// migrated from MessagePersister, name is not a typo
|
||||
private final Meter persistMessageMeter = metricRegistry.meter(name(MessagePersister.class, "persistMessage"));
|
||||
|
||||
private final MessagesDynamoDb messagesDynamoDb;
|
||||
private final MessagesCache messagesCache;
|
||||
|
@ -110,8 +114,16 @@ public class MessagesManager {
|
|||
final UUID destinationUuid,
|
||||
final long destinationDeviceId,
|
||||
final List<Envelope> messages) {
|
||||
messagesDynamoDb.store(messages, destinationUuid, destinationDeviceId);
|
||||
messagesCache.remove(destinationUuid, destinationDeviceId, messages.stream().map(message -> UUID.fromString(message.getServerGuid())).collect(Collectors.toList()));
|
||||
|
||||
final List<Envelope> nonEphemeralMessages = messages.stream()
|
||||
.filter(envelope -> !envelope.getEphemeral())
|
||||
.collect(Collectors.toList());
|
||||
|
||||
messagesDynamoDb.store(nonEphemeralMessages, destinationUuid, destinationDeviceId);
|
||||
messagesCache.remove(destinationUuid, destinationDeviceId,
|
||||
messages.stream().map(message -> UUID.fromString(message.getServerGuid())).collect(Collectors.toList()));
|
||||
|
||||
persistMessageMeter.mark(nonEphemeralMessages.size());
|
||||
}
|
||||
|
||||
public void addMessageAvailabilityListener(
|
||||
|
|
|
@ -157,7 +157,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
|
|||
}
|
||||
|
||||
private CompletableFuture<WebSocketResponseMessage> sendMessage(final Envelope message, final Optional<StoredMessageInfo> storedMessageInfo) {
|
||||
final Optional<byte[]> body = Optional.ofNullable(message.toByteArray());
|
||||
// clear ephemeral field from the envelope
|
||||
final Optional<byte[]> body = Optional.ofNullable(message.toBuilder().clearEphemeral().build().toByteArray());
|
||||
|
||||
sendMessageMeter.mark();
|
||||
sentMessageCounter.increment();
|
||||
|
@ -176,7 +177,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
|
|||
sendDeliveryReceiptFor(message);
|
||||
}
|
||||
} else {
|
||||
final List<Tag> tags = new ArrayList<>(List.of(Tag.of(STATUS_CODE_TAG, String.valueOf(response.getStatus())),
|
||||
final List<Tag> tags = new ArrayList<>(
|
||||
List.of(Tag.of(STATUS_CODE_TAG, String.valueOf(response.getStatus())),
|
||||
UserAgentTagUtil.getPlatformTag(client.getUserAgent())));
|
||||
|
||||
// TODO Remove this once we've identified the cause of message rejections from desktop clients
|
||||
|
|
Loading…
Reference in New Issue