Skip to content

Commit

Permalink
Apply spotless on entire project (opensearch-project#336)
Browse files Browse the repository at this point in the history
Applies spotless to all Java files in project.

Signed-off-by: John Mazanec <[email protected]>
  • Loading branch information
jmazanec15 authored Mar 29, 2022
1 parent 9c0b1d0 commit 02c30cb
Show file tree
Hide file tree
Showing 146 changed files with 4,960 additions and 4,105 deletions.
5 changes: 0 additions & 5 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ Please follow these formatting guidelines:
* Note that JavaDoc and block comments i.e. `/* ... */` are not formatted, but line comments i.e `// ...` are.
* There is an implicit rule that negative boolean expressions should use the form `foo == false` instead of `!foo` for better readability of the code. While this isn't strictly enforced, if might get called out in PR reviews as something to change.

In order to gradually introduce the spotless formatting, we use the
[ratchetFrom](https://github.com/diffplug/spotless/tree/main/plugin-gradle#ratchet) spotless functionality. This makes
it so only files that are changed compared to the origin branch are inspected. Because of this, ensure that your
origin branch is up to date with the plugins upstream when testing locally.

## Build

OpenSearch k-NN uses a [Gradle](https://docs.gradle.org/6.6.1/userguide/userguide.html) wrapper for its build.
Expand Down
3 changes: 1 addition & 2 deletions gradle/formatting.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ allprojects {
project.apply plugin: "com.diffplug.spotless"
spotless {
java {
ratchetFrom 'origin/main'
target '**/*.java'
target 'src/**/*.java'
removeUnusedImports()
eclipse().configFile rootProject.file('buildSrc/formatterConfig.xml')
trimTrailingWhitespace()
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/knn/common/KNNConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class KNNConstants {
// shared across library constants
public static final String DIMENSION = "dimension";
public static final String KNN_ENGINE = "engine";
public static final String KNN_METHOD= "method";
public static final String KNN_METHOD = "method";
public static final String NAME = "name";
public static final String PARAMETERS = "parameters";
public static final String METHOD_HNSW = "hnsw";
Expand Down
46 changes: 28 additions & 18 deletions src/main/java/org/opensearch/knn/index/IndexUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,12 @@ public static int getFileSizeInKB(String filePath) {
* @return ValidationException exception produced by field validation
*/
@SuppressWarnings("unchecked")
public static ValidationException validateKnnField(IndexMetadata indexMetadata, String field, int expectedDimension,
ModelDao modelDao) {
public static ValidationException validateKnnField(
IndexMetadata indexMetadata,
String field,
int expectedDimension,
ModelDao modelDao
) {
// Index metadata should not be null
if (indexMetadata == null) {
throw new IllegalArgumentException("IndexMetadata should not be null");
Expand All @@ -78,8 +82,8 @@ public static ValidationException validateKnnField(IndexMetadata indexMetadata,
}

// The mapping output *should* look like this:
// "{properties={field={type=knn_vector, dimension=8}}}"
Map<String, Object> properties = (Map<String, Object>)mappingMetadata.getSourceAsMap().get("properties");
// "{properties={field={type=knn_vector, dimension=8}}}"
Map<String, Object> properties = (Map<String, Object>) mappingMetadata.getSourceAsMap().get("properties");

if (properties == null) {
exception.addValidationError("Properties in map does not exists. This is unexpected");
Expand All @@ -106,8 +110,7 @@ public static ValidationException validateKnnField(IndexMetadata indexMetadata,
Object type = fieldMap.get("type");

if (!(type instanceof String) || !KNNVectorFieldMapper.CONTENT_TYPE.equals(type)) {
exception.addValidationError(String.format("Field \"%s\" is not of type %s.", field,
KNNVectorFieldMapper.CONTENT_TYPE));
exception.addValidationError(String.format("Field \"%s\" is not of type %s.", field, KNNVectorFieldMapper.CONTENT_TYPE));
return exception;
}

Expand All @@ -131,22 +134,25 @@ public static ValidationException validateKnnField(IndexMetadata indexMetadata,
}

if (modelDao == null) {
throw new IllegalArgumentException(String.format("Field \"%s\" uses model. modelDao cannot be null.",
field));
throw new IllegalArgumentException(String.format("Field \"%s\" uses model. modelDao cannot be null.", field));
}

ModelMetadata modelMetadata = modelDao.getMetadata(modelId);
if (modelMetadata == null) {
exception.addValidationError(String.format("Model \"%s\" for field \"%s\" does not exist.", modelId,
field));
exception.addValidationError(String.format("Model \"%s\" for field \"%s\" does not exist.", modelId, field));
return exception;
}

dimension = modelMetadata.getDimension();
if ((Integer) dimension != expectedDimension) {
exception.addValidationError(String.format("Field \"%s\" has dimension %d, which is different from " +
"dimension specified in the training request: %d", field, dimension,
expectedDimension));
exception.addValidationError(
String.format(
"Field \"%s\" has dimension %d, which is different from " + "dimension specified in the training request: %d",
field,
dimension,
expectedDimension
)
);
return exception;
}

Expand All @@ -155,8 +161,14 @@ public static ValidationException validateKnnField(IndexMetadata indexMetadata,

// If the dimension was found in training fields mapping, check that it equals the models proposed dimension.
if ((Integer) dimension != expectedDimension) {
exception.addValidationError(String.format("Field \"%s\" has dimension %d, which is different from " +
"dimension specified in the training request: %d", field, dimension, expectedDimension));
exception.addValidationError(
String.format(
"Field \"%s\" has dimension %d, which is different from " + "dimension specified in the training request: %d",
field,
dimension,
expectedDimension
)
);
return exception;
}

Expand All @@ -172,9 +184,7 @@ public static ValidationException validateKnnField(IndexMetadata indexMetadata,
* @return load parameters that will be passed to the JNI.
*/
public static Map<String, Object> getParametersAtLoading(SpaceType spaceType, KNNEngine knnEngine, String indexName) {
Map<String, Object> loadParameters = Maps.newHashMap(ImmutableMap.of(
SPACE_TYPE, spaceType.getValue()
));
Map<String, Object> loadParameters = Maps.newHashMap(ImmutableMap.of(SPACE_TYPE, spaceType.getValue()));

// For nmslib, we need to add the dynamic ef_search parameter that needs to be passed in when the
// hnsw graphs are loaded into memory
Expand Down
26 changes: 15 additions & 11 deletions src/main/java/org/opensearch/knn/index/KNNCircuitBreaker.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@
*/
public class KNNCircuitBreaker {
private static Logger logger = LogManager.getLogger(KNNCircuitBreaker.class);
public static int CB_TIME_INTERVAL = 2*60; // seconds
public static int CB_TIME_INTERVAL = 2 * 60; // seconds

private static KNNCircuitBreaker INSTANCE;
private ThreadPool threadPool;
private ClusterService clusterService;
private Client client;

private KNNCircuitBreaker() {
}
private KNNCircuitBreaker() {}

public static synchronized KNNCircuitBreaker getInstance() {
if (INSTANCE == null) {
Expand All @@ -60,10 +59,10 @@ public void initialize(ThreadPool threadPool, ClusterService clusterService, Cli
NativeMemoryCacheManager nativeMemoryCacheManager = NativeMemoryCacheManager.getInstance();
Runnable runnable = () -> {
if (nativeMemoryCacheManager.isCacheCapacityReached() && clusterService.localNode().isDataNode()) {
long currentSizeKiloBytes = nativeMemoryCacheManager.getCacheSizeInKilobytes();
long currentSizeKiloBytes = nativeMemoryCacheManager.getCacheSizeInKilobytes();
long circuitBreakerLimitSizeKiloBytes = KNNSettings.getCircuitBreakerLimit().getKb();
long circuitBreakerUnsetSizeKiloBytes = (long) ((KNNSettings.getCircuitBreakerUnsetPercentage()/100)
* circuitBreakerLimitSizeKiloBytes);
long circuitBreakerUnsetSizeKiloBytes = (long) ((KNNSettings.getCircuitBreakerUnsetPercentage() / 100)
* circuitBreakerLimitSizeKiloBytes);
/**
* Unset capacityReached flag if currentSizeBytes is less than circuitBreakerUnsetSizeBytes
*/
Expand All @@ -76,7 +75,7 @@ public void initialize(ThreadPool threadPool, ClusterService clusterService, Cli
if (KNNSettings.isCircuitBreakerTriggered() && clusterService.state().nodes().isLocalNodeElectedMaster()) {
KNNStatsRequest knnStatsRequest = new KNNStatsRequest(KNNStatsConfig.KNN_STATS.keySet());
knnStatsRequest.addStat(StatNames.CACHE_CAPACITY_REACHED.getName());
knnStatsRequest.timeout(new TimeValue(1000*10)); // 10 second timeout
knnStatsRequest.timeout(new TimeValue(1000 * 10)); // 10 second timeout

try {
KNNStatsResponse knnStatsResponse = client.execute(KNNStatsAction.INSTANCE, knnStatsRequest).get();
Expand All @@ -90,11 +89,16 @@ public void initialize(ThreadPool threadPool, ClusterService clusterService, Cli
}

if (!nodesAtMaxCapacity.isEmpty()) {
logger.info("[KNN] knn.circuit_breaker.triggered stays set. Nodes at max cache capacity: "
+ String.join(",", nodesAtMaxCapacity) + ".");
logger.info(
"[KNN] knn.circuit_breaker.triggered stays set. Nodes at max cache capacity: "
+ String.join(",", nodesAtMaxCapacity)
+ "."
);
} else {
logger.info("[KNN] Cache capacity below 75% of the circuit breaker limit for all nodes." +
" Unsetting knn.circuit_breaker.triggered flag.");
logger.info(
"[KNN] Cache capacity below 75% of the circuit breaker limit for all nodes."
+ " Unsetting knn.circuit_breaker.triggered flag."
);
KNNSettings.state().updateCircuitBreakerSettings(false);
}
} catch (Exception e) {
Expand Down
14 changes: 9 additions & 5 deletions src/main/java/org/opensearch/knn/index/KNNMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,21 @@ public Set<SpaceType> getSpaces() {
public ValidationException validate(KNNMethodContext knnMethodContext) {
List<String> errorMessages = new ArrayList<>();
if (!containsSpace(knnMethodContext.getSpaceType())) {
errorMessages.add(String.format("\"%s\" configuration does not support space type: " +
"\"%s\".", this.methodComponent.getName(), knnMethodContext.getSpaceType().getValue()));
errorMessages.add(
String.format(
"\"%s\" configuration does not support space type: " + "\"%s\".",
this.methodComponent.getName(),
knnMethodContext.getSpaceType().getValue()
)
);
}

ValidationException methodValidation = methodComponent.validate(knnMethodContext.getMethodComponent());
if (methodValidation != null) {
errorMessages.addAll(methodValidation.validationErrors());
}

if(errorMessages.isEmpty()) {
if (errorMessages.isEmpty()) {
return null;
}

Expand Down Expand Up @@ -130,7 +135,6 @@ public Map<String, Object> getAsMap(KNNMethodContext knnMethodContext) {
return parameterMap;
}


/**
* Builder for KNNMethod
*/
Expand Down Expand Up @@ -160,7 +164,7 @@ private Builder(MethodComponent methodComponent) {
* @param spaceTypes to be added
* @return Builder
*/
public Builder addSpaces(SpaceType ...spaceTypes) {
public Builder addSpaces(SpaceType... spaceTypes) {
spaces.addAll(Arrays.asList(spaceTypes));
return this;
}
Expand Down
29 changes: 15 additions & 14 deletions src/main/java/org/opensearch/knn/index/KNNMethodContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ public class KNNMethodContext implements ToXContentFragment, Writeable {

public static synchronized KNNMethodContext getDefault() {
if (defaultInstance == null) {
defaultInstance = new KNNMethodContext(KNNEngine.DEFAULT, SpaceType.DEFAULT,
new MethodComponentContext(METHOD_HNSW, Collections.emptyMap()));
defaultInstance = new KNNMethodContext(
KNNEngine.DEFAULT,
SpaceType.DEFAULT,
new MethodComponentContext(METHOD_HNSW, Collections.emptyMap())
);
}
return defaultInstance;
}
Expand Down Expand Up @@ -197,15 +200,15 @@ public static KNNMethodContext parse(Object in) {

// Interpret all map parameters as sub-MethodComponentContexts
@SuppressWarnings("unchecked")
Map<String, Object> parameters1 = ((Map<String, Object>) value).entrySet().stream().collect(Collectors.toMap(
Map.Entry::getKey, e -> {
Object v = e.getValue();
if (v instanceof Map) {
return MethodComponentContext.parse(v);
}
return v;
Map<String, Object> parameters1 = ((Map<String, Object>) value).entrySet()
.stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> {
Object v = e.getValue();
if (v instanceof Map) {
return MethodComponentContext.parse(v);
}
));
return v;
}));

parameters = parameters1;
} else {
Expand All @@ -232,10 +235,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null || getClass() != obj.getClass())
return false;
if (this == obj) return true;
if (obj == null || getClass() != obj.getClass()) return false;
KNNMethodContext other = (KNNMethodContext) obj;

EqualsBuilder equalsBuilder = new EqualsBuilder();
Expand Down
21 changes: 12 additions & 9 deletions src/main/java/org/opensearch/knn/index/KNNQueryBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,16 @@ public static KNNQueryBuilder fromXContent(XContentParser parser) throws IOExcep
} else if (AbstractQueryBuilder.NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
queryName = parser.text();
} else {
throw new ParsingException(parser.getTokenLocation(),
"[" + NAME + "] query does not support [" + currentFieldName + "]");
throw new ParsingException(
parser.getTokenLocation(),
"[" + NAME + "] query does not support [" + currentFieldName + "]"
);
}
} else {
throw new ParsingException(parser.getTokenLocation(),
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]");
throw new ParsingException(
parser.getTokenLocation(),
"[" + NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"
);
}
}
} else {
Expand Down Expand Up @@ -218,18 +222,17 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
}

if (dimension != vector.length) {
throw new IllegalArgumentException("Query vector has invalid dimension: " + vector.length +
". Dimension should be: " + dimension);
throw new IllegalArgumentException(
"Query vector has invalid dimension: " + vector.length + ". Dimension should be: " + dimension
);
}

return new KNNQuery(this.fieldName, vector, k, context.index().getName());
}

@Override
protected boolean doEquals(KNNQueryBuilder other) {
return Objects.equals(fieldName, other.fieldName) &&
Objects.equals(vector, other.vector) &&
Objects.equals(k, other.k);
return Objects.equals(fieldName, other.fieldName) && Objects.equals(vector, other.vector) && Objects.equals(k, other.k);
}

@Override
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/opensearch/knn/index/KNNScorer.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public float getMaxScore(int upTo) throws IOException {
public float score() {
assert docID() != DocIdSetIterator.NO_MORE_DOCS;
Float score = scores.get(docID());
if (score == null)
throw new RuntimeException("Null score for the docID: " + docID());
if (score == null) throw new RuntimeException("Null score for the docID: " + docID());
return score;
}

Expand All @@ -58,4 +57,3 @@ public int docID() {
return docIdsIter.docID();
}
}

Loading

0 comments on commit 02c30cb

Please sign in to comment.