PakhomovAlexander commented on code in PR #1789: URL: https://github.com/apache/ignite-3/pull/1789#discussion_r1135987033
########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/key/UnitKey.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.key; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Base64.Encoder; +import org.apache.ignite.lang.ByteArray; + +/** + * Helper for deployment units metastore keys generation. + */ +public final class UnitKey { + private static final String DEPLOY_UNIT_PREFIX = "deploy-unit."; + + private static final String UNITS_PREFIX = DEPLOY_UNIT_PREFIX + "units."; + + private UnitKey() { + + } + + /** + * Key to find all deployment units. + * + * @return Key in {@link ByteArray} format. + */ + public static ByteArray all() { + return key(null, null); + } + + /** + * Key to find all deployment units with required id. + * + * @param id Required unit id. + * @return Key in {@link ByteArray} format. + */ + public static ByteArray withId(String id) { + return key(id, null); + } + + /** + * Key for unit with required id and version. Only one unit should exist with this key. + * + * @param id Required unit id. + * @param version Required unit version. + * @return Key in {@link ByteArray} format. + */ + public static ByteArray key(String id, String version) { + StringBuilder sb = new StringBuilder(UNITS_PREFIX); + Encoder encoder = Base64.getEncoder(); + if (id != null) { + sb.append(encoder.encodeToString(id.getBytes(StandardCharsets.UTF_8))); + sb.append(":"); Review Comment: if the version is null then sb = "some.id" + ":"; Do we really need ":" in this case? ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/ListAccumulator.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Plain list accumulator. + * + * @param <T> Result value type. + */ +public class ListAccumulator<T extends Comparable<T>> implements Accumulator<List<T>> { Review Comment: Subscribers and Accumulators look really good. Don't you think we can move them to the metastorage API level instead of keeping them in deployunit module? ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/UnitStatusAccumulator.java: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import org.apache.ignite.deployment.UnitStatus; +import org.apache.ignite.deployment.UnitStatus.UnitStatusBuilder; +import org.apache.ignite.internal.deployunit.UnitMeta; +import org.apache.ignite.internal.deployunit.exception.DeploymentUnitNotFoundException; +import org.apache.ignite.internal.deployunit.key.UnitMetaSerializer; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Unit status accumulator. + */ +public class UnitStatusAccumulator implements Accumulator<UnitStatus> { + private final String id; + private UnitStatusBuilder builder; Review Comment: ```suggestion private UnitStatusBuilder builder; ``` ########## modules/rest-api/openapi/openapi.yaml: ########## @@ -386,6 +386,35 @@ paths: application/problem+json: schema: $ref: '#/components/schemas/Problem' + /management/v1/deployment/units/consistentId/{consistentId}: + get: + tags: + - deployment + description: Status of units which deployed on node. + operationId: byConsistentId + parameters: + - name: consistentId + in: path + required: true + schema: + required: + - "true" + type: string + responses: + "200": + description: All statutes returned successful. + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/UnitStatus' + "500": Review Comment: I think in case of an incorrect consistent id (non-existing one, for example) we have to return something like Not Found or Bad Request. ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/DeploymentManagerImpl.java: ########## @@ -214,7 +208,8 @@ public CompletableFuture<Void> undeployAsync(String id, Version version) { checkId(id); Objects.requireNonNull(version); - ByteArray key = new ByteArray(UNITS_PREFIX + id + ":" + version); + String version1 = version.render(); Review Comment: ```suggestion String stringVersion = version.render(); ``` ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/ListAccumulator.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Plain list accumulator. + * + * @param <T> Result value type. + */ +public class ListAccumulator<T extends Comparable<T>> implements Accumulator<List<T>> { + private final Function<Entry, T> mapper; + private final List<T> result = new ArrayList<>(); Review Comment: Should the resulting collection be thread-safe? ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/ListAccumulator.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Plain list accumulator. + * + * @param <T> Result value type. + */ +public class ListAccumulator<T extends Comparable<T>> implements Accumulator<List<T>> { + private final Function<Entry, T> mapper; + private final List<T> result = new ArrayList<>(); Review Comment: ```suggestion private final List<T> result = new ArrayList<>(); ``` ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/UnitsAccumulator.java: ########## @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import org.apache.ignite.deployment.UnitStatus; +import org.apache.ignite.deployment.UnitStatus.UnitStatusBuilder; +import org.apache.ignite.internal.deployunit.UnitMeta; +import org.apache.ignite.internal.deployunit.key.UnitMetaSerializer; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Units accumulator with filtering mechanism. + */ +public class UnitsAccumulator implements Accumulator<List<UnitStatus>> { + private final Map<String, UnitStatusBuilder> map = new HashMap<>(); Review Comment: thread safety ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/UnitStatusAccumulator.java: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import org.apache.ignite.deployment.UnitStatus; +import org.apache.ignite.deployment.UnitStatus.UnitStatusBuilder; +import org.apache.ignite.internal.deployunit.UnitMeta; +import org.apache.ignite.internal.deployunit.exception.DeploymentUnitNotFoundException; +import org.apache.ignite.internal.deployunit.key.UnitMetaSerializer; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Unit status accumulator. + */ +public class UnitStatusAccumulator implements Accumulator<UnitStatus> { + private final String id; + private UnitStatusBuilder builder; + + /** + * Constructor. + * + * @param id Identifier of required unit. + */ + public UnitStatusAccumulator(String id) { + this.id = id; + } + + @Override + public void accumulate(Entry item) { + if (builder == null) { Review Comment: same question about thread safety. ########## modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/UnitsAccumulator.java: ########## @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.ignite.internal.deployunit.metastore; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import org.apache.ignite.deployment.UnitStatus; +import org.apache.ignite.deployment.UnitStatus.UnitStatusBuilder; +import org.apache.ignite.internal.deployunit.UnitMeta; +import org.apache.ignite.internal.deployunit.key.UnitMetaSerializer; +import org.apache.ignite.internal.metastorage.Entry; + +/** + * Units accumulator with filtering mechanism. + */ +public class UnitsAccumulator implements Accumulator<List<UnitStatus>> { + private final Map<String, UnitStatusBuilder> map = new HashMap<>(); + private final Predicate<UnitMeta> filter; Review Comment: ```suggestion private final Predicate<UnitMeta> filter; ``` ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/DeploymentManagementController.java: ########## @@ -81,6 +81,13 @@ public CompletableFuture<UnitStatusDto> status(String unitId) { return deployment.statusAsync(unitId).thenApply(UnitStatusDto::fromUnitStatus); } + @Override + public CompletableFuture<Collection<UnitStatusDto>> findByConsistentId(String consistentId) { Review Comment: It would be nice to see some tests. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
