Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Make catalog async client array-aware #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Dec 16, 2019

Changes the way Catalog AsyncClient to natively understand array tasks. The benefits of the change are:

  1. Optimized storage of subtask catalog-lookup/write status.
  2. Less chances of eviction (have already seen that happen and had to bump the number of entries to 10K)
  3. Less chances of wrong configuration (if the size of the cache is less than the number of allowed array task size, eviction is guaranteed leading to starvation and stuck jobs)

Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think.

wi.progress = bitarray.NewBitSet(uint(wi.downloadRequest.Count))
}

isArray := wi.downloadRequest.Count > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment for this? Don't understand. We still want the loop to run even if the Count is 0, why? When is the count 0?

// TODO: size limit is weird to be passed here...
outputReader = ioutils.NewRemoteFileOutputReader(ctx, wi.request.dataStore,
ioutils.NewRemoteFileOutputPaths(ctx, wi.request.dataStore, indexedOutputLocation),
int64(999999999))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in bytes? 953MBs?

katrogan pushed a commit that referenced this pull request Dec 17, 2019
- in the case when a container exits as success, but writes an error file, we would exit as success. This change actually returns the correct error status
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants