Skip to content

Commit 8ccca1d

Browse files
authored
Merge pull request #304 from NASA-PDS/bugfix/342-ldd-conflict-logging
Fix false ERROR log when loading older LDD over existing newer version
2 parents ef75c78 + 5a171b8 commit 8ccca1d

7 files changed

Lines changed: 515 additions & 18 deletions

File tree

CLAUDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ The library provides a unified `Request` and `Response` interface that abstracts
114114
- Builds publish to Maven Central via Sonatype Central Portal
115115
- Requires secrets: `CENTRAL_REPOSITORY_USERNAME`, `CENTRAL_REPOSITORY_TOKEN`, `CODE_SIGNING_KEY`
116116

117+
## Coding Style
118+
119+
All Java source files must follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). Key rules:
120+
121+
- **Indentation**: 2 spaces (no tabs)
122+
- **Column limit**: 100 characters
123+
- **Braces**: always used for blocks, opening brace on same line
124+
- **Imports**: no wildcard imports; ordered as static imports first, then standard library, then third-party, then project imports, each group separated by a blank line
125+
- **Naming**: `UpperCamelCase` for classes, `lowerCamelCase` for methods and variables, `UPPER_SNAKE_CASE` for constants
126+
117127
## Critical Invariants
118128

119129
**Schema field must exist before metadata is loaded:**

src/main/java/gov/nasa/pds/registry/common/es/dao/DataLoader.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ public class DataLoader {
3838
private int printProgressSize = 500;
3939
private int batchSize = 100;
4040
private int totalRecords;
41+
// When true, 409 Conflict on "create" is treated as success (document already exists).
42+
// Safe for idempotent LDD loads; leave false for product ingestion where duplicates are errors.
43+
private boolean ignoreConflicts = false;
4144

4245
private Logger log;
4346
private ConnectionFactory conFactory;
@@ -55,9 +58,19 @@ public DataLoader(ConnectionFactory conFactory) throws Exception {
5558
}
5659

5760

61+
/**
62+
* When set to true, 409 Conflict responses on "create" operations are treated as success.
63+
* Use for idempotent loads (e.g. LDD ingestion) where a pre-existing document is acceptable.
64+
* Default is false; keep false for product ingestion.
65+
*/
66+
public void setIgnoreConflicts(boolean ignoreConflicts) {
67+
this.ignoreConflicts = ignoreConflicts;
68+
}
69+
70+
5871
/**
5972
* Set data batch size
60-
*
73+
*
6174
* @param size batch size
6275
*/
6376
public void setBatchSize(int size) {
@@ -184,8 +197,13 @@ private String loadBatch(BufferedReader fileReader, String firstLine, int retrie
184197
totalRecords += uploaded;
185198

186199
if (uploaded != numRecords) {
200+
// When ignoreConflicts=true, 409s are not counted as errors so uploaded==numRecords
201+
// and this branch is not reached. When ignoreConflicts=false (default, product
202+
// ingestion), 409s ARE counted as failures, so a shortfall here means real write
203+
// failures that should be investigated.
187204
throw new Exception(
188-
"Failed to upload all documents (" + uploaded + "/" + numRecords + ") to -dd");
205+
"Failed to upload " + (numRecords - uploaded) + "/" + numRecords + " documents to index '"
206+
+ conFactory.getIndexName() + "'. Check ERROR lines above for per-document reasons.");
189207
}
190208
return line1;
191209
} catch (UnknownHostException ex) {
@@ -306,9 +324,14 @@ private int processErrors(Response.Bulk resp, Set<String> errorLidvids,
306324
if (resp.errors()) {
307325
for (Response.Bulk.Item item : resp.items()) {
308326
if (item.error()) {
309-
if (item.operation().equals("create") && item.status() == 409) { // already exists
327+
if (item.operation().equals("create") && item.status() == 409) {
310328
todo.remove(asKey(item));
311-
numErrors++;
329+
if (ignoreConflicts) {
330+
log.debug("Document '{}' already exists in '{}', skipping (create 409).",
331+
item.id(), conFactory.getIndexName());
332+
} else {
333+
numErrors++;
334+
}
312335
} else {
313336
String message = item.reason();
314337
String sanitizedLidvid = item.id().replace('\r', ' ').replace('\n', ' '); // protect vs

src/main/java/gov/nasa/pds/registry/common/es/dao/dd/LddVersions.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
*/
1414
public class LddVersions
1515
{
16-
private static final String DEFAULT_DATE = "1965-01-01T00:00:00.000Z";
16+
public static final String DEFAULT_DATE = "1965-01-01T00:00:00.000Z";
17+
public static final Instant DEFAULT_LAST_DATE = Instant.parse(DEFAULT_DATE);
1718

1819
public Set<String> files;
1920
public Instant lastDate;

src/main/java/gov/nasa/pds/registry/common/es/service/JsonLddLoader.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public JsonLddLoader(DataDictionaryDao dao, ConnectionFactory conFact) throws Ex
4949
dtMap = new Pds2EsDataTypeMap();
5050

5151
loader = new DataLoader(conFact.clone().setIndexName(conFact.getIndexName() + "-dd"));
52+
loader.setIgnoreConflicts(true);
5253
this.dao = dao;
5354
}
5455

@@ -238,6 +239,22 @@ private String createEsDataFile(File lddFile, String lddFileName, String namespa
238239

239240
// If this LDD date is after the last stored in Elasticsearch, overwrite old records
240241
boolean overwrite = overwriteLdd(lastDate, attrParser.getLddDate());
242+
if (!overwrite && !lastDate.equals(LddVersions.DEFAULT_LAST_DATE)) {
243+
// Only log "not newer" when the date was parseable — overwriteLdd() returns false on
244+
// parse failure too, but in that case it already logged a warn about the bad date string.
245+
boolean dateWasParseable;
246+
try {
247+
LddUtils.lddDateToIsoInstant(attrParser.getLddDate());
248+
dateWasParseable = true;
249+
} catch (Exception ex) {
250+
dateWasParseable = false;
251+
}
252+
if (dateWasParseable) {
253+
log.debug("LDD {} (dated {}) is not newer than the version already loaded in the registry (dated {})."
254+
+ " Existing field definitions for namespace '{}' will be used.",
255+
lddFileName, attrParser.getLddDate(), lastDate, namespace);
256+
}
257+
}
241258

242259
// Create a writer to save LDD data in Elasticsearch JSON data file
243260
LddEsJsonWriter writer = null;

src/main/java/gov/nasa/pds/registry/common/es/service/SchemaUpdater.java

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ public void updateSchema(Set<String> fields, Map<String, String> xsds) throws Ex
103103
} catch (LddException ex) {
104104
throw ex;
105105
} catch (Exception ex) {
106-
log.error("Could not update LDD for namespace '" + prefix + "' at URI " + uri
107-
+ ": " + ex.getMessage() + ". Harvesting will continue with available field definitions.");
106+
log.warn("Could not update LDD for namespace '{}' at URI {}: {}. Harvesting will continue with available field definitions.",
107+
prefix, uri, ex.getMessage(), ex);
108108
}
109109
}
110110
}
@@ -203,17 +203,17 @@ private void updateLdd(String uri, String prefix) throws LddException {
203203
throw ex;
204204
} catch (InterruptedException ex) {
205205
Thread.currentThread().interrupt();
206-
log.error("Interrupted while downloading or loading LDD for namespace '" + prefix + "' from " + jsonUrl);
207206
if (lddInfo.isEmpty()) {
207+
log.error("Interrupted while downloading or loading LDD for namespace '{}' from {} and no previously loaded version exists.",
208+
prefix, jsonUrl);
208209
if (!forceLoad) {
209210
throw new LddException("No previously loaded LDD found for namespace '" + prefix
210211
+ "'. Cannot load products with fields from this namespace.");
211212
}
212-
log.warn("Force mode: no LDD found for namespace '" + prefix
213-
+ "'. Fields from this namespace will not be indexed.");
213+
log.warn("Force mode: no LDD found for namespace '{}'. Fields from this namespace will not be indexed.", prefix);
214214
} else {
215-
log.warn("Will use previously loaded field definitions for namespace '" + prefix
216-
+ "' from " + lddInfo.files);
215+
log.warn("Interrupted while loading LDD {} for namespace '{}'. Will use previously loaded field definitions from {}.",
216+
schemaFileName, prefix, lddInfo.files);
217217
}
218218
return;
219219
} catch (Exception ex) {
@@ -236,18 +236,17 @@ private void updateLdd(String uri, String prefix) throws LddException {
236236
}
237237
if (mirrorSuccess) return;
238238
}
239-
log.error("Failed to download or load LDD for namespace '" + prefix + "' from " + jsonUrl
240-
+ ": " + ExceptionUtils.getMessage(ex));
241239
if (lddInfo.isEmpty()) {
240+
log.error("Failed to download or load LDD for namespace '{}' from {}: {}",
241+
prefix, jsonUrl, ExceptionUtils.getMessage(ex));
242242
if (!forceLoad) {
243243
throw new LddException("No previously loaded LDD found for namespace '" + prefix
244244
+ "'. Cannot load products with fields from this namespace.");
245245
}
246-
log.warn("Force mode: no LDD found for namespace '" + prefix
247-
+ "'. Fields from this namespace will not be indexed.");
246+
log.warn("Force mode: no LDD found for namespace '{}'. Fields from this namespace will not be indexed.", prefix);
248247
} else {
249-
log.warn("Will use previously loaded field definitions for namespace '" + prefix
250-
+ "' from " + lddInfo.files);
248+
log.warn("Failed to load LDD {} for namespace '{}': {}. Will use previously loaded field definitions from {}.",
249+
schemaFileName, prefix, ExceptionUtils.getMessage(ex), lddInfo.files);
251250
}
252251
} finally {
253252
lddFile.delete();

0 commit comments

Comments
 (0)