-
Notifications
You must be signed in to change notification settings - Fork 34
Fix race condition in subnet deletion API #663
base: master
Are you sure you want to change the base?
Changes from 9 commits
9645772
183c36a
4896515
f170e1e
bcbf7b1
efc5b6e
c785697
d213c03
0f56454
4a2a382
7f93fe3
8d6dd0e
b19d428
539d0c0
6a9a1c6
19430b2
1d7ef7e
cd1fec6
e079a44
4831ad6
3c378e0
4cb934e
ad62ef0
f756eef
8160d4a
639a0cd
85de8fb
d6ed075
e57abe4
1b63054
fe81008
016aaf5
118dd53
7d9a2e5
3884a44
0b5cd11
2ae9045
257ef71
c303e0b
46b79f8
c7809b3
70e9251
06bc414
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,16 +17,20 @@ free of charge, to any person obtaining a copy of this software and associated d | |
|
||
import com.futurewei.alcor.common.db.CacheException; | ||
import com.futurewei.alcor.common.db.ICache; | ||
import com.futurewei.alcor.common.db.Transaction; | ||
import com.futurewei.alcor.common.exception.DatabasePersistenceException; | ||
import com.futurewei.alcor.common.exception.ResourceNotFoundException; | ||
import com.futurewei.alcor.common.stats.DurationStatistics; | ||
import com.futurewei.alcor.vpcmanager.dao.VpcRepository; | ||
import com.futurewei.alcor.vpcmanager.service.VpcDatabaseService; | ||
import com.futurewei.alcor.web.entity.vpc.VpcEntity; | ||
import com.futurewei.alcor.web.entity.vpc.VpcWebJson; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
@Service | ||
|
@@ -42,7 +46,7 @@ public class VpcDatabaseServiceImpl implements VpcDatabaseService { | |
public VpcEntity getByVpcId(String vpcId) { | ||
try { | ||
return this.vpcRepository.findItem(vpcId); | ||
}catch (Exception e) { | ||
} catch (Exception e) { | ||
return null; | ||
} | ||
} | ||
|
@@ -75,6 +79,38 @@ public void deleteVpc(String id) throws CacheException { | |
this.vpcRepository.deleteItem(id); | ||
} | ||
|
||
@Override | ||
public VpcEntity deleteSubnetIdInVpc(String vpcId, String subnetId) throws Exception { | ||
|
||
VpcEntity currentVpcState = null; | ||
|
||
try (Transaction tx = this.vpcRepository.startTransaction()) { | ||
|
||
currentVpcState = getByVpcId(vpcId); | ||
if (currentVpcState == null) { | ||
throw new ResourceNotFoundException("Vpc not found : " + vpcId); | ||
} | ||
|
||
List<String> subnets = currentVpcState.getSubnets(); | ||
if (subnets == null || !subnets.contains(subnetId)) { | ||
return currentVpcState; | ||
} | ||
|
||
subnets.remove(subnetId); | ||
currentVpcState.setSubnets(subnets); | ||
addVpc(currentVpcState); | ||
|
||
tx.commit(); | ||
} catch (ResourceNotFoundException | DatabasePersistenceException | CacheException e) { | ||
throw e; | ||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
throw e; | ||
} | ||
|
||
return currentVpcState; | ||
} | ||
|
||
@Override | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good but there is a convention about the naming. The convention is to call someCache.getTransaction().start(), .commit(), .rollback() instead of wrapping the transaction calls in a method and calling it. It would be better if this code follows the convention. Another change is required for this to actually work. Ignite configuration declare that this cache must have atomicity mode of TRANSACTIONAL. Here is the declatration.
In kubernetes/services/ignite_config.xml there is this section under which there is tag . The above declaration should be added under the tag.Every one needs this update to the ignite config file to not run into this issue, in addition the iginte database should be deleted. (default is <your_ignite_home>/work). If anyone has valuable data in the ignite DB, let me know we can delete just this cache. If there are any questions about this extra work, ping me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pkommoju Thanks. We have updated the transaction naming and added a new cacheConfiguration for VpcRepo. |
||
@DurationStatistics | ||
public ICache<String, VpcEntity> getCache() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Best to actually try to start Ignite with this change, one can never be sure about XML nestings and Ignite's naming oddities.