Skip to content

Commit

Permalink
[controller][vpj] Revert changes to MultiSchemaResponse/SchemaRoutes …
Browse files Browse the repository at this point in the history
…that impacts controllerClient#getAllReplicationMetadataSchemas() (linkedin#719)

As a side-effect in PR: linkedin#704 I changed the Json property in controller class SchemaRoutes and MultiSchemaResponse to try to clean up API naming to make it makes more sense - ID-RMD_VERSION_ID. This leads to breaking changes in controller, it needs controller deployment to release VPJ and it might have break the API JSON serialization/deserialization.
To make it easy to release the new TTL repush AAWC feature, this PR reverts the part of the change, so it no longer requires controller deployment. I will try to make another attempt apart from this PR to gracefully deprecate improper naming of JSON property.
  • Loading branch information
sixpluszero authored Oct 31, 2023
1 parent a5be207 commit 077419d
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void saveSchemaResponseToDisk(MultiSchemaResponse.Schema[] schemas, boolean isRm
// Path for RMD schema is /<rmdSchemaDir>/<valueSchemaId>_<rmdSchemaId>
// Path for Value schema is /<valueSchemaDir>/<valueSchemaId>
String schemaFileName = isRmdSchema
? rmdSchemaDir + SEPARATOR + schema.getId() + UNDERSCORE + schema.getRmdSchemaId()
? rmdSchemaDir + SEPARATOR + schema.getRmdValueSchemaId() + UNDERSCORE + schema.getId()
: valueSchemaDir + SEPARATOR + schema.getId();
Path schemaPath = new Path(schemaFileName);
if (!fs.exists(schemaPath)) {
Expand All @@ -107,7 +107,7 @@ void saveSchemaResponseToDisk(MultiSchemaResponse.Schema[] schemas, boolean isRm
}
LOGGER.info(
"Finished writing schema {} onto disk",
isRmdSchema ? schema.getId() + "-" + schema.getRmdSchemaId() : schema.getId());
isRmdSchema ? schema.getRmdValueSchemaId() + "-" + schema.getId() : schema.getId());
} else {
throw new IllegalStateException(String.format("The schema path %s already exists.", schemaPath));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private MultiSchemaResponse.Schema[] generateRmdSchemas(int n) {
MultiSchemaResponse.Schema[] response = new MultiSchemaResponse.Schema[n];
for (int i = 1; i <= n; i++) {
MultiSchemaResponse.Schema schema = new MultiSchemaResponse.Schema();
schema.setRmdSchemaId(i);
schema.setRmdValueSchemaId(i);
schema.setDerivedSchemaId(i);
schema.setId(i);
schema.setSchemaStr(RMD_SCHEMA.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private MultiSchemaResponse.Schema[] generateRmdSchemas(int n) {
MultiSchemaResponse.Schema[] response = new MultiSchemaResponse.Schema[n];
for (int i = 1; i <= n; i++) {
MultiSchemaResponse.Schema schema = new MultiSchemaResponse.Schema();
schema.setRmdSchemaId(i);
schema.setRmdValueSchemaId(i);
schema.setDerivedSchemaId(i);
schema.setId(i);
schema.setSchemaStr(RMD_SCHEMA.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static class Schema {
*/
private int derivedSchemaId = SchemaData.INVALID_VALUE_SCHEMA_ID;

private int rmdSchemaId = SchemaData.INVALID_VALUE_SCHEMA_ID;
private int rmdValueSchemaId = SchemaData.INVALID_VALUE_SCHEMA_ID;

private String schemaStr;

Expand Down Expand Up @@ -60,12 +60,12 @@ public void setSchemaStr(String schemaStr) {
this.schemaStr = schemaStr;
}

public int getRmdSchemaId() {
return rmdSchemaId;
public int getRmdValueSchemaId() {
return rmdValueSchemaId;
}

public void setRmdSchemaId(int rmdSchemaId) {
this.rmdSchemaId = rmdSchemaId;
public void setRmdValueSchemaId(int rmdSchemaId) {
this.rmdValueSchemaId = rmdSchemaId;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public static MultiSchemaResponse.Schema getSupersetSchemaFromSchemaResponse(
if (schema.getDerivedSchemaId() != SchemaData.INVALID_VALUE_SCHEMA_ID) {
continue;
}
if (schema.getRmdSchemaId() != SchemaData.INVALID_VALUE_SCHEMA_ID) {
if (schema.getRmdValueSchemaId() != SchemaData.INVALID_VALUE_SCHEMA_ID) {
continue;
}
return schema;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ public void controllerClientCanGetStoreReplicationMetadataSchema() {
String expectedSchema =
"{\"type\":\"record\",\"name\":\"string_MetadataRecord\",\"namespace\":\"com.linkedin.venice\",\"fields\":[{\"name\":\"timestamp\",\"type\":[\"long\"],\"doc\":\"timestamp when the full record was last updated\",\"default\":0},{\"name\":\"replication_checkpoint_vector\",\"type\":{\"type\":\"array\",\"items\":\"long\"},\"doc\":\"high watermark remote checkpoints which touched this record\",\"default\":[]}]}";
assertEquals(schemaResponse.getSchemas()[0].getSchemaStr(), expectedSchema);
assertEquals(schemaResponse.getSchemas()[0].getRmdSchemaId(), 1);
assertEquals(schemaResponse.getSchemas()[0].getRmdValueSchemaId(), 1);
assertEquals(schemaResponse.getSchemas()[0].getId(), 1);
} finally {
deleteStores(storeName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,9 @@ public Route getAllReplicationMetadataSchemas(Admin admin) {
int cur = 0;
for (RmdSchemaEntry entry: valueSchemaEntries) {
schemas[cur] = new MultiSchemaResponse.Schema();
schemas[cur].setId(entry.getValueSchemaID());
schemas[cur].setId(entry.getId());
schemas[cur].setSchemaStr(entry.getSchema().toString());
schemas[cur].setRmdSchemaId(entry.getId());
schemas[cur].setRmdValueSchemaId(entry.getValueSchemaID());
++cur;
}
responseObject.setSchemas(schemas);
Expand Down

0 comments on commit 077419d

Please sign in to comment.