Yingyi Bu has posted comments on this change. Change subject: Add a dataset rebalance REST API. ......................................................................
Patch Set 11: (9 comments) https://asterix-gerrit.ics.uci.edu/#/c/1768/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RebalanceApiServlet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RebalanceApiServlet.java: PS10, Line 2: * 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.asterix.api.http.server; : > ? Done PS10, Line 106: > don't flush. Done https://asterix-gerrit.ics.uci.edu/#/c/1768/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/utils/RebalanceUtil.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/utils/RebalanceUtil.java: PS10, Line 3: * 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. : */ : p > there are extra spaces and asterix chars in the files you've added in this Done PS10, Line 1: /* : * 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.asterix.utils; : > ? Done PS10, Line 92: Dataset targetDataset; > lock released after getting the dataset. someone could drop it at anytime!! Done. In theory, this requires lock-upgrade, as we need the write lock for the dataset in the last step of rebalance, i.e., switching the dataset from the rebalance source to the rebalance target. I work around that by release the read lock before the last step and re-check the dataset existence in rebalanceSwitch() PS10, Line 138: > fix comment Done PS10, Line 164: reates the re > what does it mean if the node group was found?? what if it doesn't contain Done https://asterix-gerrit.ics.uci.edu/#/c/1768/10/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/transaction/DatasetIdFactory.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/transaction/DatasetIdFactory.java: PS10, Line 39: > and then what? we need to recycle these somehow, right? The current behavior is already recycling when integer overflow. My change just monotonically improves -- it recycles from the initial start id so that metadata dataset will not be destroyed. PS10, Line 43: } : : p > this is just * -1 ? if so, why not change it? I don't think it is the most It is XOR, not *-1. * -1 is error prone because Integer.MIN_VALUE * -1 = Integer.MIN_VALUE -- To view, visit https://asterix-gerrit.ics.uci.edu/1768 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibda35252031fc4940972f0f19bbf796cadfa53d6 Gerrit-PatchSet: 11 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
