You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by ma...@apache.org on 2016/03/17 03:10:37 UTC
[6/7] kylin git commit: KYLIN-1502 When cube is not empty,
only signature consistent cube desc updates are allowed
KYLIN-1502 When cube is not empty, only signature consistent cube desc updates are allowed
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/f5030b64
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/f5030b64
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/f5030b64
Branch: refs/heads/master
Commit: f5030b6426708ebef687787bd21b2dce9795af50
Parents: 98ac225
Author: Hongbin Ma <ma...@apache.org>
Authored: Wed Mar 16 23:31:43 2016 +0800
Committer: Hongbin Ma <ma...@apache.org>
Committed: Wed Mar 16 23:42:24 2016 +0800
----------------------------------------------------------------------
.../org/apache/kylin/cube/CubeDescManager.java | 2 +-
.../org/apache/kylin/cube/model/CubeDesc.java | 13 ++++
.../kylin/rest/controller/CubeController.java | 69 ++++++++++----------
.../apache/kylin/rest/service/CubeService.java | 33 ++++------
.../kylin/rest/service/CubeServiceTest.java | 2 +-
5 files changed, 63 insertions(+), 56 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kylin/blob/f5030b64/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
----------------------------------------------------------------------
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
index 56667b6..33a6830 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
@@ -58,7 +58,7 @@ public class CubeDescManager {
if (r != null) {
return r;
}
-
+
synchronized (CubeDescManager.class) {
r = CACHE.get(config);
if (r != null) {
http://git-wip-us.apache.org/repos/asf/kylin/blob/f5030b64/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
----------------------------------------------------------------------
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
index 704af60..4a019ca 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java
@@ -438,6 +438,13 @@ public class CubeDesc extends RootPersistentEntity {
return "CubeDesc [name=" + name + "]";
}
+ /**
+ * this method is to prevent malicious metadata change by checking the saved signature
+ * with the calculated signature.
+ *
+ * if you're comparing two cube desc prefer to use consistentWith()
+ * @return
+ */
public boolean checkSignature() {
if (KylinVersion.isCompatibleWith(getVersion()) && !KylinVersion.isSignatureCompatibleWith(getVersion())) {
logger.info("checkSignature on {} is skipped as the its version is {} (not signature compatible but compatible) ", getName(), getVersion());
@@ -453,6 +460,12 @@ public class CubeDesc extends RootPersistentEntity {
return calculated.equals(saved);
}
+ public boolean consistentWith(CubeDesc another) {
+ if (another == null)
+ return false;
+ return this.calculateSignature().equals(another.calculateSignature());
+ }
+
public String calculateSignature() {
MessageDigest md;
try {
http://git-wip-us.apache.org/repos/asf/kylin/blob/f5030b64/server/src/main/java/org/apache/kylin/rest/controller/CubeController.java
----------------------------------------------------------------------
diff --git a/server/src/main/java/org/apache/kylin/rest/controller/CubeController.java b/server/src/main/java/org/apache/kylin/rest/controller/CubeController.java
index bc00795..410b2bd 100644
--- a/server/src/main/java/org/apache/kylin/rest/controller/CubeController.java
+++ b/server/src/main/java/org/apache/kylin/rest/controller/CubeController.java
@@ -18,7 +18,7 @@
package org.apache.kylin.rest.controller;
-import java.io.*;
+import java.io.IOException;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Iterator;
@@ -92,13 +92,13 @@ public class CubeController extends BasicController {
@Autowired
private JobService jobService;
- @RequestMapping(value = "", method = {RequestMethod.GET})
+ @RequestMapping(value = "", method = { RequestMethod.GET })
@ResponseBody
public List<CubeInstance> getCubes(@RequestParam(value = "cubeName", required = false) String cubeName, @RequestParam(value = "modelName", required = false) String modelName, @RequestParam(value = "projectName", required = false) String projectName, @RequestParam(value = "limit", required = false) Integer limit, @RequestParam(value = "offset", required = false) Integer offset) {
return cubeService.getCubes(cubeName, projectName, modelName, limit, offset);
}
- @RequestMapping(value = "/{cubeName}", method = {RequestMethod.GET})
+ @RequestMapping(value = "/{cubeName}", method = { RequestMethod.GET })
@ResponseBody
public CubeInstance getCube(@PathVariable String cubeName) {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeName);
@@ -108,7 +108,6 @@ public class CubeController extends BasicController {
return cube;
}
-
/**
* Get hive SQL of the cube
*
@@ -117,7 +116,7 @@ public class CubeController extends BasicController {
* @throws UnknownHostException
* @throws IOException
*/
- @RequestMapping(value = "/{cubeName}/segs/{segmentName}/sql", method = {RequestMethod.GET})
+ @RequestMapping(value = "/{cubeName}/segs/{segmentName}/sql", method = { RequestMethod.GET })
@ResponseBody
public GeneralResponse getSql(@PathVariable String cubeName, @PathVariable String segmentName) {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeName);
@@ -139,7 +138,7 @@ public class CubeController extends BasicController {
* @param notifyList
* @throws IOException
*/
- @RequestMapping(value = "/{cubeName}/notify_list", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/notify_list", method = { RequestMethod.PUT })
@ResponseBody
public void updateNotifyList(@PathVariable String cubeName, @RequestBody List<String> notifyList) {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeName);
@@ -157,7 +156,7 @@ public class CubeController extends BasicController {
}
- @RequestMapping(value = "/{cubeName}/cost", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/cost", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance updateCubeCost(@PathVariable String cubeName, @RequestParam(value = "cost") int cost) {
try {
@@ -169,7 +168,7 @@ public class CubeController extends BasicController {
}
}
- @RequestMapping(value = "/{cubeName}/coprocessor", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/coprocessor", method = { RequestMethod.PUT })
@ResponseBody
public Map<String, Boolean> updateCubeCoprocessor(@PathVariable String cubeName, @RequestParam(value = "force") String force) {
try {
@@ -187,7 +186,7 @@ public class CubeController extends BasicController {
*
* @throws IOException
*/
- @RequestMapping(value = "/{cubeName}/segs/{segmentName}/refresh_lookup", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/segs/{segmentName}/refresh_lookup", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance rebuildLookupSnapshot(@PathVariable String cubeName, @PathVariable String segmentName, @RequestParam(value = "lookupTable") String lookupTable) {
try {
@@ -205,7 +204,7 @@ public class CubeController extends BasicController {
* @return
* @throws IOException
*/
- @RequestMapping(value = "/{cubeName}/rebuild", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/rebuild", method = { RequestMethod.PUT })
@ResponseBody
public JobInstance rebuild(@PathVariable String cubeName, @RequestBody JobBuildRequest jobBuildRequest) {
try {
@@ -222,7 +221,7 @@ public class CubeController extends BasicController {
}
}
- @RequestMapping(value = "/{cubeName}/disable", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/disable", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance disableCube(@PathVariable String cubeName) {
try {
@@ -240,7 +239,7 @@ public class CubeController extends BasicController {
}
}
- @RequestMapping(value = "/{cubeName}/purge", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/purge", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance purgeCube(@PathVariable String cubeName) {
try {
@@ -258,8 +257,7 @@ public class CubeController extends BasicController {
}
}
-
- @RequestMapping(value = "/{cubeName}/clone", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/clone", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance cloneCube(@PathVariable String cubeName, @RequestBody CubeRequest cubeRequest) {
String newCubeName = cubeRequest.getCubeName();
@@ -271,10 +269,9 @@ public class CubeController extends BasicController {
}
CubeDesc cubeDesc = cube.getDescriptor();
CubeDesc newCubeDesc = CubeDesc.getCopyOf(cubeDesc);
-
newCubeDesc.setName(newCubeName);
- CubeInstance newCube = null;
+ CubeInstance newCube;
try {
newCube = cubeService.createCubeAndDesc(newCubeName, project, newCubeDesc);
@@ -288,8 +285,7 @@ public class CubeController extends BasicController {
}
-
- @RequestMapping(value = "/{cubeName}/enable", method = {RequestMethod.PUT})
+ @RequestMapping(value = "/{cubeName}/enable", method = { RequestMethod.PUT })
@ResponseBody
public CubeInstance enableCube(@PathVariable String cubeName) {
try {
@@ -306,7 +302,7 @@ public class CubeController extends BasicController {
}
}
- @RequestMapping(value = "/{cubeName}", method = {RequestMethod.DELETE})
+ @RequestMapping(value = "/{cubeName}", method = { RequestMethod.DELETE })
@ResponseBody
public void deleteCube(@PathVariable String cubeName) {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeName);
@@ -330,7 +326,7 @@ public class CubeController extends BasicController {
* @return Table metadata array
* @throws IOException
*/
- @RequestMapping(value = "", method = {RequestMethod.POST})
+ @RequestMapping(value = "", method = { RequestMethod.POST })
@ResponseBody
public CubeRequest saveCubeDesc(@RequestBody CubeRequest cubeRequest) {
@@ -368,37 +364,40 @@ public class CubeController extends BasicController {
* @throws JsonProcessingException
* @throws IOException
*/
- @RequestMapping(value = "", method = {RequestMethod.PUT})
+ @RequestMapping(value = "", method = { RequestMethod.PUT })
@ResponseBody
public CubeRequest updateCubeDesc(@RequestBody CubeRequest cubeRequest) throws JsonProcessingException {
//update cube
CubeDesc desc = deserializeCubeDesc(cubeRequest);
- CubeDesc oldCubeDesc = null;
+ CubeDesc oldCubeDesc;
+ boolean isCubeDescFreeEditable;
if (desc == null) {
return cubeRequest;
}
// Check if the cube is editable
- if (!cubeService.isCubeDescEditable(desc)) {
- String error = "Purge the related cube before editing its desc. Desc name: " + desc.getName();
- updateRequest(cubeRequest, false, error);
- return cubeRequest;
- }
+ isCubeDescFreeEditable = cubeService.isCubeDescFreeEditable(desc);
- //cube renaming:
+ //cube renaming is not allowed
if (!cubeRequest.getCubeName().equalsIgnoreCase(CubeService.getCubeNameFromDesc(desc.getName()))) {
- deleteCube(cubeRequest.getCubeName());
- saveCubeDesc(cubeRequest);
+ String error = "Cube Desc renaming is not allowed: " + desc.getName();
+ updateRequest(cubeRequest, false, error);
+ return cubeRequest;
}
String projectName = (null == cubeRequest.getProject()) ? ProjectInstance.DEFAULT_PROJECT_NAME : cubeRequest.getProject();
try {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeRequest.getCubeName());
oldCubeDesc = cube.getDescriptor();
- desc = cubeService.updateCubeAndDesc(cube, desc, projectName);
-
+ if (isCubeDescFreeEditable || oldCubeDesc.consistentWith(desc)) {
+ desc = cubeService.updateCubeAndDesc(cube, desc, projectName);
+ } else {
+ logger.warn("Won't update the cube desc due to inconsistency");
+ updateRequest(cubeRequest, false, "CubeDesc " + desc.getName() + " is inconsistent with existing. Try purge that cube first or avoid updating key cube desc fields.");
+ return cubeRequest;
+ }
} catch (AccessDeniedException accessDeniedException) {
throw new ForbiddenException("You don't have right to update this cube.");
} catch (Exception e) {
@@ -424,7 +423,7 @@ public class CubeController extends BasicController {
* @return true
* @throws IOException
*/
- @RequestMapping(value = "/{cubeName}/hbase", method = {RequestMethod.GET})
+ @RequestMapping(value = "/{cubeName}/hbase", method = { RequestMethod.GET })
@ResponseBody
public List<HBaseResponse> getHBaseInfo(@PathVariable String cubeName) {
List<HBaseResponse> hbase = new ArrayList<HBaseResponse>();
@@ -461,7 +460,7 @@ public class CubeController extends BasicController {
return hbase;
}
- @RequestMapping(value = "/{cubeName}/segments", method = {RequestMethod.POST})
+ @RequestMapping(value = "/{cubeName}/segments", method = { RequestMethod.POST })
@ResponseBody
public CubeSegmentRequest appendSegment(@PathVariable String cubeName, @RequestBody CubeSegmentRequest cubeSegmentRequest) {
CubeInstance cube = cubeService.getCubeManager().getCube(cubeName);
@@ -567,7 +566,7 @@ public class CubeController extends BasicController {
*/
private String omitMessage(List<String> errors) {
StringBuffer buffer = new StringBuffer();
- for (Iterator<String> iterator = errors.iterator(); iterator.hasNext(); ) {
+ for (Iterator<String> iterator = errors.iterator(); iterator.hasNext();) {
String string = (String) iterator.next();
buffer.append(string);
buffer.append("\n");
http://git-wip-us.apache.org/repos/asf/kylin/blob/f5030b64/server/src/main/java/org/apache/kylin/rest/service/CubeService.java
----------------------------------------------------------------------
diff --git a/server/src/main/java/org/apache/kylin/rest/service/CubeService.java b/server/src/main/java/org/apache/kylin/rest/service/CubeService.java
index 0c57d00..6ef796c 100644
--- a/server/src/main/java/org/apache/kylin/rest/service/CubeService.java
+++ b/server/src/main/java/org/apache/kylin/rest/service/CubeService.java
@@ -171,27 +171,23 @@ public class CubeService extends BasicService {
throw new InternalErrorException("The cube named " + cubeName + " already exists");
}
+ if (getCubeDescManager().getCubeDesc(desc.getName()) != null) {
+ throw new InternalErrorException("The cube desc named " + desc.getName() + " already exists");
+ }
+
String owner = SecurityContextHolder.getContext().getAuthentication().getName();
- CubeDesc createdDesc = null;
- CubeInstance createdCube = null;
+ CubeDesc createdDesc;
+ CubeInstance createdCube;
- boolean isNew = false;
- if (getCubeDescManager().getCubeDesc(desc.getName()) == null) {
- createdDesc = getCubeDescManager().createCubeDesc(desc);
- isNew = true;
- } else {
- createdDesc = getCubeDescManager().updateCubeDesc(desc);
- }
+ createdDesc = getCubeDescManager().createCubeDesc(desc);
if (!createdDesc.getError().isEmpty()) {
- if (isNew) {
- getCubeDescManager().removeCubeDesc(createdDesc);
- }
+ getCubeDescManager().removeCubeDesc(createdDesc);
throw new InternalErrorException(createdDesc.getError().get(0));
}
try {
- int cuboidCount = CuboidCLI.simulateCuboidGeneration(createdDesc,false);
+ int cuboidCount = CuboidCLI.simulateCuboidGeneration(createdDesc, false);
logger.info("New cube " + cubeName + " has " + cuboidCount + " cuboids");
} catch (Exception e) {
getCubeDescManager().removeCubeDesc(createdDesc);
@@ -256,13 +252,12 @@ public class CubeService extends BasicService {
}
try {
- if (!cube.getDescriptor().checkSignature()) {
- logger.info("Releasing all segments due to cube desc conflict");
- this.releaseAllSegments(cube);
+ if (!cube.getDescriptor().consistentWith(desc)) {
+ throw new IllegalStateException("cube's desc is not consistent with the new desc");
}
CubeDesc updatedCubeDesc = getCubeDescManager().updateCubeDesc(desc);
- int cuboidCount = CuboidCLI.simulateCuboidGeneration(updatedCubeDesc,false);
+ int cuboidCount = CuboidCLI.simulateCuboidGeneration(updatedCubeDesc, false);
logger.info("Updated cube " + cube.getName() + " has " + cuboidCount + " cuboids");
ProjectManager projectManager = getProjectManager();
@@ -290,7 +285,7 @@ public class CubeService extends BasicService {
accessService.clean(cube, true);
}
- public boolean isCubeDescEditable(CubeDesc cd) {
+ public boolean isCubeDescFreeEditable(CubeDesc cd) {
List<CubeInstance> cubes = getCubeManager().getCubesByDesc(cd.getName());
for (CubeInstance cube : cubes) {
if (cube.getSegments().size() != 0) {
@@ -300,6 +295,7 @@ public class CubeService extends BasicService {
}
return true;
}
+
public static String getCubeDescNameFromCube(String cubeName) {
return cubeName + DESC_SUFFIX;
}
@@ -312,7 +308,6 @@ public class CubeService extends BasicService {
}
}
-
/**
* Stop all jobs belonging to this cube and clean out all segments
*
http://git-wip-us.apache.org/repos/asf/kylin/blob/f5030b64/server/src/test/java/org/apache/kylin/rest/service/CubeServiceTest.java
----------------------------------------------------------------------
diff --git a/server/src/test/java/org/apache/kylin/rest/service/CubeServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/CubeServiceTest.java
index d86935e..f93c04f 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/CubeServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/CubeServiceTest.java
@@ -51,7 +51,7 @@ public class CubeServiceTest extends ServiceTestBase {
List<CubeInstance> cubes = cubeService.getCubes(null, null, null, null, null);
Assert.assertNotNull(cubes);
CubeInstance cube = cubes.get(0);
- cubeService.isCubeDescEditable(cube.getDescriptor());
+ cubeService.isCubeDescFreeEditable(cube.getDescriptor());
cubes = cubeService.getCubes(null, null, null, 1, 0);
Assert.assertTrue(cubes.size() == 1);