Skip to content

Commit

Permalink
Use yyyyMMddHHmmssSSS formatted datetime instead of UUID for index id (
Browse files Browse the repository at this point in the history
…#696)

* Use yyyyMMddHHmmssSSS formatted datetime instead of UUID for index id

* Add back test, spotlessApply

* Revert changes to RestoreIncrementalCommand

* Change setDateTime parameter to setId

* Reverted change in test

* Update grpc-gateway

* Always use UTC time for index id
  • Loading branch information
sarthakn7 authored Aug 21, 2024
1 parent 10c0c53 commit a996905
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 33 deletions.
4 changes: 2 additions & 2 deletions clientlib/src/main/proto/yelp/nrtsearch/luceneserver.proto
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ service ReplicationServer {
/* Input to createIndex */
message CreateIndexRequest {
string indexName = 1; // name of the index to be created. [a-zA-Z0-9]*
// Set if state/data already exists in the data backend. Provides the UUID to identify and load the index.
// Set if state/data already exists in the data backend. Provides the yyyyMMddHHmmssSSS formatted date-time string to identify and load the index.
string existsWithId = 2;
// Optional initial index settings
IndexSettings settings = 3;
Expand Down Expand Up @@ -1132,7 +1132,7 @@ message IndexStateInfo {
}

message IndexGlobalState {
// Unique identifier for index (UUID)
// Unique identifier for index (yyyyMMddHHmmssSSS formatted date-time string)
string id = 1;
// If index should be started
bool started = 2;
Expand Down
4 changes: 2 additions & 2 deletions grpc-gateway/luceneserver.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions grpc-gateway/luceneserver.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2281,7 +2281,7 @@
"description": "Application specific response metadata. Must be set in the first response\nfor streaming APIs."
}
},
"description": "Message that represents an arbitrary HTTP body. It should only be used for\npayload formats that can't be represented as JSON, such as raw binary or\nan HTML page.\n\n\nThis message can be used both in streaming and non-streaming API methods in\nthe request as well as the response.\n\nIt can be used as a top-level request field, which is convenient if one\nwants to extract parameters from either the URL or HTTP template into the\nrequest fields and also want access to the raw HTTP body.\n\nExample:\n\n message GetResourceRequest {\n // A unique request id.\n string request_id = 1;\n\n // The raw HTTP body is bound to this field.\n google.api.HttpBody http_body = 2;\n }\n\n service ResourceService {\n rpc GetResource(GetResourceRequest) returns (google.api.HttpBody);\n rpc UpdateResource(google.api.HttpBody) returns\n (google.protobuf.Empty);\n }\n\nExample with streaming methods:\n\n service CaldavService {\n rpc GetCalendar(stream google.api.HttpBody)\n returns (stream google.api.HttpBody);\n rpc UpdateCalendar(stream google.api.HttpBody)\n returns (stream google.api.HttpBody);\n }\n\nUse of this type only changes how the request and response bodies are\nhandled, all other features will continue to work unchanged."
"description": "Message that represents an arbitrary HTTP body. It should only be used for\npayload formats that can't be represented as JSON, such as raw binary or\nan HTML page.\n\n\nThis message can be used both in streaming and non-streaming API methods in\nthe request as well as the response.\n\nIt can be used as a top-level request field, which is convenient if one\nwants to extract parameters from either the URL or HTTP template into the\nrequest fields and also want access to the raw HTTP body.\n\nExample:\n\n message GetResourceRequest {\n // A unique request id.\n string request_id = 1;\n\n // The raw HTTP body is bound to this field.\n google.api.HttpBody http_body = 2;\n\n }\n\n service ResourceService {\n rpc GetResource(GetResourceRequest)\n returns (google.api.HttpBody);\n rpc UpdateResource(google.api.HttpBody)\n returns (google.protobuf.Empty);\n\n }\n\nExample with streaming methods:\n\n service CaldavService {\n rpc GetCalendar(stream google.api.HttpBody)\n returns (stream google.api.HttpBody);\n rpc UpdateCalendar(stream google.api.HttpBody)\n returns (stream google.api.HttpBody);\n\n }\n\nUse of this type only changes how the request and response bodies are\nhandled, all other features will continue to work unchanged."
},
"googlerpcStatus": {
"type": "object",
Expand Down Expand Up @@ -2714,7 +2714,7 @@
},
"existsWithId": {
"type": "string",
"description": "Set if state/data already exists in the data backend. Provides the UUID to identify and load the index."
"description": "Set if state/data already exists in the data backend. Provides the yyyyMMddHHmmssSSS formatted date-time string to identify and load the index."
},
"settings": {
"$ref": "#/definitions/luceneserverIndexSettings",
Expand Down Expand Up @@ -5760,7 +5760,7 @@
"description": "The longitude in degrees. It must be in the range [-180.0, +180.0]."
}
},
"description": "An object representing a latitude/longitude pair. This is expressed as a pair\nof doubles representing degrees latitude and degrees longitude. Unless\nspecified otherwise, this must conform to the\n\u003ca href=\"http://www.unoosa.org/pdf/icg/2012/template/WGS_84.pdf\"\u003eWGS84\nstandard\u003c/a\u003e. Values must be within normalized ranges."
"description": "An object that represents a latitude/longitude pair. This is expressed as a\npair of doubles to represent degrees latitude and degrees longitude. Unless\nspecified otherwise, this must conform to the\n\u003ca href=\"http://www.unoosa.org/pdf/icg/2012/template/WGS_84.pdf\"\u003eWGS84\nstandard\u003c/a\u003e. Values must be within normalized ranges."
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,6 @@ public InputStream nextElement() {
}

private String getTmpName() {
return UUID.randomUUID().toString() + TMP_SUFFIX;
return UUID.randomUUID() + TMP_SUFFIX;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package com.yelp.nrtsearch.server.luceneserver.state;

import static com.yelp.nrtsearch.server.utils.IndexIdUtil.generateIndexId;

import com.google.common.annotations.VisibleForTesting;
import com.yelp.nrtsearch.server.backup.Archiver;
import com.yelp.nrtsearch.server.config.IndexStartConfig;
Expand Down Expand Up @@ -49,7 +51,6 @@
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import org.apache.lucene.util.IOUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -166,7 +167,7 @@ protected IndexStateManager createIndexStateManager(
* Generate a unique id to identify an index instance. Protected to allow injection for testing.
*/
protected String getIndexId() {
return UUID.randomUUID().toString();
return generateIndexId();
}

@VisibleForTesting
Expand Down
41 changes: 41 additions & 0 deletions src/main/java/com/yelp/nrtsearch/server/utils/IndexIdUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2024 Yelp Inc.
*
* Licensed 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 com.yelp.nrtsearch.server.utils;

import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;

public class IndexIdUtil {

private static final DateTimeFormatter INDEX_ID_FORMATTER =
DateTimeFormatter.ofPattern("yyyyMMddHHmmssSSS");

/** Generate a unique index id based on the current time formatted as yyyyMMddHHmmssSSS. */
public static String generateIndexId() {
return INDEX_ID_FORMATTER.format(LocalDateTime.now(ZoneId.of("UTC")));
}

/** Check if the given string is a valid index id. */
public static boolean isIndexId(String indexId) {
try {
INDEX_ID_FORMATTER.parse(indexId);
return true;
} catch (Exception e) {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.yelp.nrtsearch.server.luceneserver.state.BackendGlobalState;
import com.yelp.nrtsearch.server.luceneserver.state.StateUtils;
import com.yelp.nrtsearch.server.luceneserver.state.backend.RemoteStateBackend;
import com.yelp.nrtsearch.server.utils.IndexIdUtil;
import com.yelp.nrtsearch.tools.nrt_utils.legacy.incremental.IncrementalCommandUtils;
import java.util.concurrent.Callable;
import picocli.CommandLine;
Expand Down Expand Up @@ -70,9 +71,10 @@ public class UpdateGlobalIndexStateCommand implements Callable<Integer> {
private String credsProfile;

@CommandLine.Option(
names = {"--setUUID"},
description = "If specified, update index UUID to this value")
private String uuid;
names = {"--setId"},
description =
"If specified, update index id to this value. Should match format yyyyMMddHHmmssSSS")
private String dateTimeString;

@CommandLine.Option(
names = {"--setStarted"},
Expand All @@ -94,16 +96,16 @@ void setS3Client(AmazonS3 s3Client) {
}

@VisibleForTesting
static boolean validateParams(String started, String uuid) {
static boolean validateParams(String started, String dateTimeString) {
if (started != null) {
if (!started.equalsIgnoreCase("true") && !started.equalsIgnoreCase("false")) {
System.out.println("setStarted must be one of 'true' or 'false'");
return false;
}
}
if (uuid != null) {
if (!IncrementalCommandUtils.isUUID(uuid)) {
System.out.println("Invalid UUID format: " + uuid);
if (dateTimeString != null) {
if (!IndexIdUtil.isIndexId(dateTimeString)) {
System.out.println("Invalid date time format: " + dateTimeString);
return false;
}
}
Expand All @@ -112,7 +114,7 @@ static boolean validateParams(String started, String uuid) {

@Override
public Integer call() throws Exception {
if (!validateParams(started, uuid)) {
if (!validateParams(started, dateTimeString)) {
return 1;
}
if (s3Client == null) {
Expand Down Expand Up @@ -145,8 +147,9 @@ public Integer call() throws Exception {
IndexGlobalState indexGlobalState = globalStateInfo.getIndicesOrThrow(indexName);
boolean updated = false;

if (uuid != null) {
String updatedIndexResource = BackendGlobalState.getUniqueIndexName(indexName, uuid);
if (dateTimeString != null) {
String updatedIndexResource =
BackendGlobalState.getUniqueIndexName(indexName, dateTimeString);
String updatedIndexDataResource =
IncrementalCommandUtils.getIndexDataResource(updatedIndexResource);
String updatedIndexStateResource =
Expand All @@ -156,15 +159,15 @@ public Integer call() throws Exception {
long stateVersion =
versionManager.getLatestVersionNumber(serviceName, updatedIndexStateResource);
if (dataVersion == -1 || stateVersion == -1) {
System.out.println("Missing blessed resources for new uuid: " + uuid);
System.out.println("Missing blessed resources for new index id: " + dateTimeString);
System.out.println(
"Data resource: " + updatedIndexDataResource + ", version: " + dataVersion);
System.out.println(
"State resource: " + updatedIndexStateResource + ", version: " + stateVersion);
return 1;
}

indexGlobalState = indexGlobalState.toBuilder().setId(uuid).build();
indexGlobalState = indexGlobalState.toBuilder().setId(dateTimeString).build();
updated = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -50,6 +51,8 @@
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -64,6 +67,7 @@
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;

public class BackendGlobalStateTest {

Expand Down Expand Up @@ -1328,4 +1332,16 @@ public void testGetResolvedReplicationPort() throws IOException {
backendGlobalState.replicationStarted(100);
assertEquals(100, backendGlobalState.getReplicationPort());
}

@Test
public void testGetIndexId() {
BackendGlobalState mockBackendGlobalState = mock(BackendGlobalState.class);
when(mockBackendGlobalState.getIndexId()).thenCallRealMethod();
LocalDateTime mockTime = LocalDateTime.of(2024, 8, 20, 12, 34, 56, 789000000);
try (MockedStatic<LocalDateTime> mockLocalDateTime = mockStatic(LocalDateTime.class)) {
mockLocalDateTime.when(() -> LocalDateTime.now(ZoneId.of("UTC"))).thenReturn(mockTime);
String indexId = mockBackendGlobalState.getIndexId();
assertEquals("20240820123456789", indexId);
}
}
}
53 changes: 53 additions & 0 deletions src/test/java/com/yelp/nrtsearch/server/utils/IndexIdUtilTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright 2024 Yelp Inc.
*
* Licensed 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 com.yelp.nrtsearch.server.utils;

import static org.junit.Assert.*;
import static org.mockito.Mockito.mockStatic;

import java.time.LocalDateTime;
import java.time.ZoneId;
import org.junit.Test;
import org.mockito.MockedStatic;

public class IndexIdUtilTest {

@Test
public void testGenerateIndexId() {
LocalDateTime mockTime = LocalDateTime.of(2024, 8, 20, 12, 34, 56, 789000000);
try (MockedStatic<LocalDateTime> mockLocalDateTime = mockStatic(LocalDateTime.class)) {
mockLocalDateTime.when(() -> LocalDateTime.now(ZoneId.of("UTC"))).thenReturn(mockTime);
String indexId = IndexIdUtil.generateIndexId();
assertEquals("20240820123456789", indexId);
}
}

@Test
public void testIsIndexId() {
assertTrue(IndexIdUtil.isIndexId("20240820123456789"));
assertTrue(IndexIdUtil.isIndexId("19701010000000000"));
assertTrue(IndexIdUtil.isIndexId("20391229233759999"));

assertFalse(IndexIdUtil.isIndexId("20241329233759999"));
assertFalse(IndexIdUtil.isIndexId("20241232233759999"));
assertFalse(IndexIdUtil.isIndexId("20241231243759999"));
assertFalse(IndexIdUtil.isIndexId("09d9c9e4-483e-4a90-9c4F-D342c8da1210"));
assertFalse(IndexIdUtil.isIndexId("09d9c9e4-483e-4a90-D342c8da1210"));
assertFalse(IndexIdUtil.isIndexId("other_file"));
assertFalse(IndexIdUtil.isIndexId("_3.cfs"));
assertFalse(IndexIdUtil.isIndexId("segments"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.yelp.nrtsearch.server.grpc.Mode;
import com.yelp.nrtsearch.server.grpc.TestServer;
import java.io.IOException;
import java.util.UUID;
import org.junit.After;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -124,7 +123,7 @@ public void testUpdateIndexUUID() throws IOException {
"--serviceName=" + SERVICE_NAME,
"--bucketName=" + TEST_BUCKET,
"--indexName=test_index",
"--setUUID=" + firstIndexId);
"--setId=" + firstIndexId);
assertEquals(0, exitCode);
server.restart();
assertTrue(server.isStarted("test_index"));
Expand Down Expand Up @@ -192,7 +191,7 @@ public void testIndexNotInState() throws IOException {
}

@Test
public void testIndexUUIDNotInBackend() throws IOException {
public void testIndexIDNotInBackend() throws IOException {
TestServer server = getTestServer();
server.startPrimaryIndex("test_index", -1, null);
String indexId = server.getGlobalState().getIndexStateManager("test_index").getIndexId();
Expand All @@ -203,7 +202,7 @@ public void testIndexUUIDNotInBackend() throws IOException {
"--serviceName=" + SERVICE_NAME,
"--bucketName=" + TEST_BUCKET,
"--indexName=not_index",
"--setUUID=" + UUID.randomUUID());
"--setId=20240820123456789");
assertEquals(1, exitCode);
server.restart();

Expand All @@ -223,15 +222,12 @@ public void testValidateStarted() {
}

@Test
public void testValidateIndexUUID() {
public void testValidateIndexID() {
assertTrue(UpdateGlobalIndexStateCommand.validateParams(null, null));
assertTrue(
UpdateGlobalIndexStateCommand.validateParams(null, "d5401128-7aed-427c-8dc3-70a3e24c7c9a"));
assertTrue(
UpdateGlobalIndexStateCommand.validateParams(null, "d5401128-7AED-427c-8dc3-70a3e24c7c9a"));
assertTrue(UpdateGlobalIndexStateCommand.validateParams(null, "20240820123456789"));
assertTrue(UpdateGlobalIndexStateCommand.validateParams(null, "19701010000000000"));
assertFalse(UpdateGlobalIndexStateCommand.validateParams(null, ""));
assertFalse(UpdateGlobalIndexStateCommand.validateParams(null, "invalid"));
assertFalse(
UpdateGlobalIndexStateCommand.validateParams(null, "d5401128-7AED-427c-70a3e24c7c9a"));
assertFalse(UpdateGlobalIndexStateCommand.validateParams(null, "20241329233759999"));
}
}

0 comments on commit a996905

Please sign in to comment.