Skip to content

Commit

Permalink
issue #3437 allow reindex to be forced after database migration
Browse files Browse the repository at this point in the history
Signed-off-by: Robin Arnold <robin.arnold@ibm.com>
  • Loading branch information
punktilious committed Jun 8, 2022
1 parent 4de35a5 commit 40e2454
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 25 deletions.
6 changes: 3 additions & 3 deletions build/migration/bin/6_current-reindex.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ run_reindex(){
DATE_ISO=$(date +%Y-%m-%dT%H:%M:%SZ)
status=$(curl -k -X POST -o reindex.json -i -w '%{http_code}' -u 'fhiruser:change-password' 'https://localhost:9443/fhir-server/api/v4/$reindex' \
-H 'Content-Type: application/fhir+json' -H 'X-FHIR-TENANT-ID: default' \
-d "{\"resourceType\": \"Parameters\",\"parameter\":[{\"name\":\"resourceCount\",\"valueInteger\":100},{\"name\":\"tstamp\",\"valueString\":\"${DATE_ISO}\"}]}")
-d "{\"resourceType\": \"Parameters\",\"parameter\":[{\"name\":\"resourceCount\",\"valueInteger\":100},{\"name\":\"tstamp\",\"valueString\":\"${DATE_ISO}\"},{\"name\":\"force\",\"valueBoolean\":true}]}")
echo "Status: ${status}"

while [ $status -ne 200 ]
Expand Down Expand Up @@ -57,7 +57,7 @@ run_reindex(){
fi
status=$(curl -k -X POST -o reindex.json -i -w '%{http_code}' -u 'fhiruser:change-password' 'https://localhost:9443/fhir-server/api/v4/$reindex' \
-H 'Content-Type: application/fhir+json' -H 'X-FHIR-TENANT-ID: default' \
-d "{\"resourceType\": \"Parameters\",\"parameter\":[{\"name\":\"resourceCount\",\"valueInteger\":100},{\"name\":\"tstamp\",\"valueString\":\"${DATE_ISO}\"}]}")
-d "{\"resourceType\": \"Parameters\",\"parameter\":[{\"name\":\"resourceCount\",\"valueInteger\":100},{\"name\":\"tstamp\",\"valueString\":\"${DATE_ISO}\"},{\"name\":\"force\",\"valueBoolean\":true}]}")
echo "Status: ${status}"
done
fi
Expand All @@ -77,4 +77,4 @@ run_reindex "${1}"
popd > /dev/null

# EOF
###############################################################################
###############################################################################
1 change: 1 addition & 0 deletions docs/src/pages/guides/FHIRSearchConfiguration.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ By default, the operation will select 10 resources and re-extract their search p
|----|----|-----------|
|`tstamp`|string|Reindex only resources not previously reindexed since this timestamp. Format as a date YYYY-MM-DD or time YYYY-MM-DDTHH:MM:SSZ.|
|`resourceCount`|integer|The maximum number of resources to reindex in this call. If this number is too large, the processing time might exceed the transaction timeout and fail.|
|`force`|boolean|Force the parameters to be replaced even if the parameter hash matches. This is only required following a schema migration which changes how the parameters are stored in the database.|

The IBM FHIR Server tracks when a resource was last reindexed and only resources with a reindex_tstamp value less than the given tstamp parameter will be processed. When a resource is reindexed, its reindex_tstamp is set to the given tstamp value. In most cases, using the current date (for example "2020-10-27") is the best option for this value.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2607,7 +2607,7 @@ public boolean isOffloadingSupported() {

@Override
public int reindex(FHIRPersistenceContext context, OperationOutcome.Builder operationOutcomeResult, java.time.Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException {
String resourceLogicalId, boolean force) throws FHIRPersistenceException {
final String METHODNAME = "reindex";
log.entering(CLASSNAME, METHODNAME);

Expand Down Expand Up @@ -2679,7 +2679,7 @@ public int reindex(FHIRPersistenceContext context, OperationOutcome.Builder oper
rir.setDeleted(false); // just to be clear
Class<? extends Resource> resourceTypeClass = getResourceType(rir.getResourceType());
reindexDAO.setPersistenceContext(context);
updateParameters(rir, resourceTypeClass, existingResourceDTO, reindexDAO, operationOutcomeResult);
updateParameters(rir, resourceTypeClass, existingResourceDTO, reindexDAO, operationOutcomeResult, force);

// result is only 0 if getResourceToReindex doesn't give us anything because this indicates
// there's nothing left to do
Expand Down Expand Up @@ -2738,10 +2738,11 @@ public int reindex(FHIRPersistenceContext context, OperationOutcome.Builder oper
* @param existingResourceDTO the existing resource DTO
* @param reindexDAO the reindex resource DAO
* @param operationOutcomeResult the operation outcome result
* @param force
* @throws Exception
*/
public <T extends Resource> void updateParameters(ResourceIndexRecord rir, Class<T> resourceTypeClass, com.ibm.fhir.persistence.jdbc.dto.Resource existingResourceDTO,
ReindexResourceDAO reindexDAO, OperationOutcome.Builder operationOutcomeResult) throws Exception {
ReindexResourceDAO reindexDAO, OperationOutcome.Builder operationOutcomeResult, boolean force) throws Exception {
if (existingResourceDTO != null && !existingResourceDTO.isDeleted()) {
T existingResource = this.convertResourceDTO(existingResourceDTO, resourceTypeClass, null);

Expand All @@ -2751,7 +2752,7 @@ public <T extends Resource> void updateParameters(ResourceIndexRecord rir, Class
// Compare the hash of the extracted parameters with the hash in the index record.
// If hash in the index record is not null and it matches the hash of the extracted parameters, then no need to replace the
// extracted search parameters in the database tables for this resource, which helps with performance during reindex.
if (rir.getParameterHash() == null || !rir.getParameterHash().equals(searchParameters.getParameterHashB64())) {
if (force || rir.getParameterHash() == null || !rir.getParameterHash().equals(searchParameters.getParameterHashB64())) {
reindexDAO.updateParameters(rir.getResourceType(), searchParameters.getParameters(), searchParameters.getParameterHashB64(), rir.getLogicalId(), rir.getLogicalResourceId());
} else {
log.fine(() -> "Skipping update of unchanged parameters for FHIR Resource '" + rir.getResourceType() + "/" + rir.getLogicalId() + "'");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,12 @@ default boolean isOffloadingSupported() {
* @param indexIds list of index IDs of resources to reindex, or null
* @param resourceLogicalId resourceType/logicalId value of a specific resource to reindex, or null;
* this parameter is ignored if the indexIds parameter value is non-null
* @param force if true, always replace the stored parameters
* @return count of the number of resources reindexed by this call
* @throws FHIRPersistenceException
*/
int reindex(FHIRPersistenceContext context, OperationOutcome.Builder operationOutcomeResult, java.time.Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException;
String resourceLogicalId, boolean force) throws FHIRPersistenceException;

/**
* Special function for high speed export of resource payloads. The process
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public OperationOutcome getHealth() throws FHIRPersistenceException {

@Override
public int reindex(FHIRPersistenceContext context, OperationOutcome.Builder oob, Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException {
String resourceLogicalId, boolean force) throws FHIRPersistenceException {
return 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,11 +490,12 @@ Resource doInvoke(FHIROperationContext operationContext, String resourceTypeName
* @param indexIds list of index IDs of resources to reindex, or null
* @param resourceLogicalId resourceType (e.g. "Patient"), or resourceType/logicalId a specific resource (e.g. "Patient/abc123"), to reindex, or null;
* this parameter is ignored if the indexIds parameter value is non-null
* @param force if true, ignore parameter hash and always replace the parameters
* @return count of the number of resources reindexed by this call
* @throws Exception
*/
int doReindex(FHIROperationContext operationContext, OperationOutcome.Builder operationOutcomeResult, Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws Exception;
String resourceLogicalId, boolean force) throws Exception;

/**
* Invoke the FHIR Persistence erase operation for a specific instance of the erase.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2611,13 +2611,13 @@ private void setOperationContextProperties(FHIROperationContext operationContext

@Override
public int doReindex(FHIROperationContext operationContext, OperationOutcome.Builder operationOutcomeResult, Instant tstamp,
List<Long> indexIds, String resourceLogicalId) throws Exception {
List<Long> indexIds, String resourceLogicalId, boolean force) throws Exception {
int result = 0;
// Since the try logic is slightly different in the code paths, we want to dispatch to separate methods to simplify the logic.
if (indexIds == null) {
result = doReindexSingle(operationOutcomeResult, tstamp, resourceLogicalId);
result = doReindexSingle(operationOutcomeResult, tstamp, resourceLogicalId, force);
} else {
result = doReindexList(operationOutcomeResult, tstamp, indexIds);
result = doReindexList(operationOutcomeResult, tstamp, indexIds, force);
}
return result;
}
Expand All @@ -2628,10 +2628,11 @@ public int doReindex(FHIROperationContext operationContext, OperationOutcome.Bui
* @param operationOutcomeResult
* @param tstamp
* @param indexIds
* @param force
* @return
* @throws Exception
*/
public int doReindexList(OperationOutcome.Builder operationOutcomeResult, Instant tstamp, List<Long> indexIds) throws Exception {
public int doReindexList(OperationOutcome.Builder operationOutcomeResult, Instant tstamp, List<Long> indexIds, boolean force) throws Exception {
// If the indexIds are empty or null, then it's not properly formed.
if (indexIds == null || indexIds.isEmpty()) {
throw new IllegalArgumentException("No indexIds sent to the $reindex list method");
Expand Down Expand Up @@ -2684,7 +2685,7 @@ public int doReindexList(OperationOutcome.Builder operationOutcomeResult, Instan
txn.begin();
try {
FHIRPersistenceContext persistenceContext = null;
result += persistence.reindex(persistenceContext, operationOutcomeResult, tstamp, subListIndexIds, null);
result += persistence.reindex(persistenceContext, operationOutcomeResult, tstamp, subListIndexIds, null, force);
} catch (FHIRPersistenceDataAccessException x) {
// At this point, the transaction is marked for rollback
if (x.isTransactionRetryable() && ++attempt <= TX_ATTEMPTS) {
Expand Down Expand Up @@ -2722,10 +2723,11 @@ public int doReindexList(OperationOutcome.Builder operationOutcomeResult, Instan
* @param operationOutcomeResult
* @param tstamp
* @param resourceLogicalId
* @param force
* @return
* @throws Exception
*/
public int doReindexSingle(OperationOutcome.Builder operationOutcomeResult, Instant tstamp, String resourceLogicalId) throws Exception {
public int doReindexSingle(OperationOutcome.Builder operationOutcomeResult, Instant tstamp, String resourceLogicalId, boolean force) throws Exception {
int result = 0;
// handle some retries in case of deadlock exceptions
final int TX_ATTEMPTS = 5;
Expand All @@ -2735,7 +2737,7 @@ public int doReindexSingle(OperationOutcome.Builder operationOutcomeResult, Inst
txn.begin();
try {
FHIRPersistenceContext persistenceContext = null;
result = persistence.reindex(persistenceContext, operationOutcomeResult, tstamp, null, resourceLogicalId);
result = persistence.reindex(persistenceContext, operationOutcomeResult, tstamp, null, resourceLogicalId, force);
attempt = TX_ATTEMPTS; // end the retry loop
} catch (FHIRPersistenceDataAccessException x) {
if (x.isTransactionRetryable() && attempt < TX_ATTEMPTS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public String generateResourceId() {

@Override
public int reindex(FHIRPersistenceContext context, Builder operationOutcomeResult, java.time.Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException {
String resourceLogicalId, boolean force) throws FHIRPersistenceException {
return 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,8 @@ public int reindex(
Builder operationOutcomeResult,
java.time.Instant tstamp,
List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException {
String resourceLogicalId,
boolean force) throws FHIRPersistenceException {
throw new UnsupportedOperationException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public OperationOutcome getHealth() throws FHIRPersistenceException {

@Override
public int reindex(FHIRPersistenceContext context, OperationOutcome.Builder oob, Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws FHIRPersistenceException {
String resourceLogicalId, boolean force) throws FHIRPersistenceException {
return 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public FHIRPersistenceTransaction getTransaction() throws Exception {

@Override
public int doReindex(FHIROperationContext operationContext, Builder operationOutcomeResult, Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws Exception {
String resourceLogicalId, boolean force) throws Exception {
return 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ public FHIRPersistenceTransaction getTransaction() throws Exception {

@Override
public int doReindex(FHIROperationContext operationContext, Builder operationOutcomeResult, Instant tstamp, List<Long> indexIds,
String resourceLogicalId) throws Exception {
String resourceLogicalId, boolean force) throws Exception {
throw new AssertionError("Unused");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class ReindexOperation extends AbstractOperation {
private static final String PARAM_INDEX_IDS = "indexIds";
private static final String PARAM_RESOURCE_COUNT = "resourceCount";
private static final String PARAM_RESOURCE_LOGICAL_ID = "resourceLogicalId";
private static final String PARAM_FORCE = "force";

// The max number of resources we allow to be processed by one request
private static final int MAX_RESOURCE_COUNT = 1000;
Expand Down Expand Up @@ -84,6 +85,7 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
List<Long> indexIds = null;
int resourceCount = 10;
String resourceLogicalId = null;
boolean force = false;

boolean hasSpecificResourceType = false;
if (resourceType != null) {
Expand Down Expand Up @@ -137,6 +139,14 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
}
resourceCount = val;
}
} else if (PARAM_FORCE.equals(parameter.getName().getValue())) {
Boolean val = parameter.getValue().as(com.ibm.fhir.model.type.Boolean.class).getValue();
if (val != null) {
if (val.booleanValue()) {
logger.info("Forcing reindex, even if parameter hash is the same");
force = true;
}
}
} else if (PARAM_RESOURCE_LOGICAL_ID.equals(parameter.getName().getValue())) {
if (hasSpecificResourceType) {
throw FHIROperationUtil.buildExceptionWithIssue("resourceLogicalId already specified using call to Operation on Type or Instance", IssueType.INVALID);
Expand All @@ -161,12 +171,12 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
int totalProcessed = 0;
if (indexIds != null) {
// All resources in one transaction
totalProcessed = resourceHelper.doReindex(operationContext, result, tstamp, indexIds, null);
totalProcessed = resourceHelper.doReindex(operationContext, result, tstamp, indexIds, null, force);
} else {
int processed = 1;
// One resource per transaction
for (int i=0; i<resourceCount && processed > 0; i++) {
processed = resourceHelper.doReindex(operationContext, result, tstamp, null, resourceLogicalId);
processed = resourceHelper.doReindex(operationContext, result, tstamp, null, resourceLogicalId, force);
totalProcessed += processed;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
"max": "1",
"documentation": "Reindex only the specified resource or resources of the given resource type when no id is provided. Format as Patient/abc123 or Patient. If indexIds is specified, this parameter is not used.",
"type": "string"
},
{
"name": "force",
"use": "in",
"min": 0,
"max": "1",
"documentation": "When true, always replace the parameters even if the parameter hash matches. This is typically used when a schema migration step changes structure used to stored parameters in the database.",
"type": "boolean"
}
]
}
}

0 comments on commit 40e2454

Please sign in to comment.