Skip to content

Commit 36c5122

Browse files
authored
Merge pull request #39 from picoded/thread-safety-debugging
Thread safety debugging
2 parents fc93d3d + 478836a commit 36c5122

2 files changed

Lines changed: 35 additions & 9 deletions

File tree

src/main/java/picoded/dstack/mongodb/MongoDBStack.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,14 @@ public static String getFullConnectionURL_primary(GenericConvertMap<String, Obje
106106
String protocol = config.getString("protocol", "mongodb");
107107
String user = config.getString("user", null);
108108
String pass = config.getString("pass", null);
109-
String host = config.getString("host", "localhost");
109+
String host = config.getString("host", null);
110110
int port = config.getInt("port", 27017);
111111

112+
// Safety check
113+
if (host == null || host.isEmpty()) {
114+
throw new IllegalArgumentException("Missing valid host setting for MongoDB connection");
115+
}
116+
112117
// Hanlding of option string
113118
GenericConvertMap<String, Object> optMap = config.getGenericConvertStringMap("opt",
114119
defaultOptJson);
@@ -257,7 +262,7 @@ public MongoDBStack(GenericConvertMap<String, Object> inConfig) {
257262
// ------
258263

259264
// Get the full_url
260-
String full_url = getFullConnectionURL_primary(inConfig);
265+
String full_url = getFullConnectionURL_primary(dbConfig);
261266

262267
// Lets build using the stable API settings
263268
ServerApi serverApi = ServerApi.builder().version(ServerApiVersion.V1).build();
@@ -275,7 +280,7 @@ public MongoDBStack(GenericConvertMap<String, Object> inConfig) {
275280
// ------
276281

277282
// Null check for secondary connection
278-
String config_sec_mode = config.getString("sec_mode", null);
283+
String config_sec_mode = dbConfig.getString("sec_mode", null);
279284
if (config_sec_mode == null) {
280285
sec_mode = null;
281286
sec_client_conn = null;
@@ -285,7 +290,7 @@ public MongoDBStack(GenericConvertMap<String, Object> inConfig) {
285290
sec_mode = config_sec_mode.trim().toUpperCase();
286291

287292
// lets get the secondary connection
288-
full_url = getFullConnectionURL_secondary(inConfig);
293+
full_url = getFullConnectionURL_secondary(dbConfig);
289294
serverApi = ServerApi.builder().version(ServerApiVersion.V1).build();
290295
settings = MongoClientSettings.builder()
291296
.applyConnectionString(new ConnectionString(full_url)).serverApi(serverApi).build();

src/main/java/picoded/dstack/stack/ProviderConfig.java

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
import java.util.List;
55
import java.util.Map;
66
import java.util.concurrent.ConcurrentHashMap;
7+
import java.util.logging.Level;
8+
import java.util.logging.Logger;
79

10+
import picoded.core.conv.ConvertJSON;
811
import picoded.core.struct.GenericConvertHashMap;
912
import picoded.core.struct.GenericConvertList;
1013
import picoded.core.struct.GenericConvertMap;
@@ -47,10 +50,15 @@ public class ProviderConfig {
4750
//
4851
//--------------------------------------------------------------------------
4952

53+
// Logger to use, for config file warnings
54+
private static final Logger LOGGER = Logger.getLogger(ProviderConfig.class.getName());
55+
5056
/**
5157
* Load the provider config with provider list
5258
**/
5359
public ProviderConfig(List<Object> inConfigList) {
60+
providerConfigMap = new HashMap<>();
61+
providerStackMap = new ConcurrentHashMap<>();
5462
loadConfigArray(inConfigList);
5563
}
5664

@@ -63,7 +71,7 @@ public ProviderConfig(List<Object> inConfigList) {
6371
/**
6472
* Stores the internal config mapping by its name
6573
**/
66-
protected final Map<String, GenericConvertMap<String, Object>> providerConfigMap = new HashMap<>();;
74+
protected final Map<String, GenericConvertMap<String, Object>> providerConfigMap;
6775

6876
/**
6977
* Loads a configuration array of backend providers for dstack, into the provider map
@@ -116,7 +124,7 @@ protected GenericConvertMap<String, Object> getStackConfig(String name) {
116124
/**
117125
* Stores the respective stack providers
118126
*/
119-
protected final ConcurrentHashMap<String, CoreStack> providerStackMap = new ConcurrentHashMap<>();
127+
protected final ConcurrentHashMap<String, CoreStack> providerStackMap;
120128

121129
/**
122130
* Get the stack of the provider specified by the name,
@@ -133,7 +141,7 @@ public CoreStack getProviderStack(String name) {
133141
return cache;
134142
}
135143

136-
synchronized (providerStackMap) {
144+
synchronized (this) {
137145
// Check the cache again (avoid race condition)
138146
cache = providerStackMap.get(name);
139147
if (cache != null) {
@@ -143,17 +151,30 @@ public CoreStack getProviderStack(String name) {
143151
// Cache not found, get config to initialize a new stack
144152
GenericConvertMap<String, Object> providerConfig = getStackConfig(name);
145153
if (providerConfig == null) {
154+
LOGGER.log(Level.SEVERE, "Unknown provider name, config not found : " + name);
146155
throw new IllegalArgumentException("Unknown provider name, config not found : " + name);
147156
}
148157

158+
// Log the setup
159+
String type = providerConfig.getString("type");
160+
LOGGER.info("Setting DStack provider backend : " + name + " (" + type + ")");
161+
149162
// Initialization of stack and store into cache
150-
cache = initStack(providerConfig.getString("type"), providerConfig);
163+
try {
164+
cache = initStack(type, providerConfig);
165+
} catch (Exception e) {
166+
// Log the error, as this is easily missed into an API error
167+
LOGGER.log(Level.SEVERE, "Error while setting DStack provider : " + name + " (" + type
168+
+ ")", e);
169+
throw new RuntimeException(e);
170+
}
171+
172+
// Save it into cache
151173
providerStackMap.put(name, cache);
152174

153175
// Return result
154176
return cache;
155177
}
156-
157178
}
158179

159180
/**

0 commit comments

Comments
 (0)