Concurrent sequences of the Adapter calls

[dhilt]

In order to provide the ui-scroll Workflow consistency, all the Adapter calls should be run in sequence. Basically this requirement is fulfilled with the help of internal Adapter methods promisification and explicit Adapter.relax calls:

await adapter.relax(); // wait for the Scroller is relaxed
await adapter.append(items1); // wait for new items are appended and the Scroller is relaxed
await adapter.append(items2); // again...
...

The more complex use case as follows:

doAppendAndScroll = async (items) => {
  await adapter.relax();
  await adapter.append(items);
  await adapter.fix({ scrollPosition: Infinity });
}

// sequence within sequence
await doAppendAndScroll(items1);
await doAppendAndScroll(items2);
...

But what if doAppendAndScroll method can be invoked randomly: by user click, by socket events, by timer... In this case, we may fall into a situation where two or more doAppendAndScroll method calls appear in the same call stack. Like the following

// two sequences running in parallel
doAppendAndScroll(items1);
doAppendAndScroll(items2);
...

This will break the flow, because we can't provide outer sequence with only Inner "await" protections. The question is

• could it be handled at the lib' end (by new wrappers for the Adapter.relax method or in any other way)?
• can we develop an appropriate technique to handle this issue on the App side?

(Relates to issues #187, #219.)

[dhilt]

Speaking of the App side solution, let's consider following approaches.

1. Flag and setTimeout

let run = false;
doAppendAndScroll = async (items) => {
  if (run) {
    return setTimeout(() => doAppendAndScroll(items));
  }
  run = true;
  await adapter.relax();
  await adapter.append(items);
  await adapter.fix({ scrollPosition: Infinity });
  run = false;
}

doAppendAndScroll({ items: [item5] });
doAppendAndScroll({ items: [item6] });
doAppendAndScroll({ items: [item7] });
...

This moves each concurrent call to a separate call stack, but this does not guarantee the order. Delayed calls will be invoked randomly, and appending, say, [5, 6, 7] items may result in [5, 7, 6]. Not to mention setTimeout itself.

2. Array of concurrent calls

Instead of flag approach, we may accumulate concurrent calls and re-invoke them each time the previous call is done. The array will be automatically reduced per each iteration.

let run = null; // : Function[] | null
const doAppendAndScroll = async (items) => {
  if (run) {
    run.push(() => doAppendAndScroll(items));
    return;
  }
  run = [];
  await adapter.relax();
  await adapter.append(items);
  await adapter.fix({ scrollPosition: Infinity });
  const _run = [...(run || [])];
  run = null;
  _run.forEach(call => call()); // re-invoking the reduced list of concurrent calls
};

This seems more accurate and stable.

[IgorVNC]

Thanks @dhilt for raising the issue and proposed solutions

Regarding App side solution - there is one issue with the proposed solutions. The issue is not only with append/prepend, but with all existing API.

For example, it can be concurrent calls of the following API:

• append/prepend
• reload
• fix

so it's all should be synchronized together

Regarding possible solutions, I have the following in my mind:

Lib side

1) To fix/advance 'relax' method, so it will fire a callback when isLoading is 100% true

2) new 'safe' API.

Instead of doing:

doAppend() {
    this.datasource.adapter.relax(() => {
          this.datasource.adapter.append([msg]);
      });
 }

we can use a 'safe' API which will manage everything inside:

doAppend() {
    this.datasource.adapter.safeAppend([msg]);
 }

App side

1) To create some method/wrapper, which will resolve the promise once the lib is relaxed and isLoading=false. Then, wrap every vs adapter API call inside this method

[dhilt]

Basically the technique I suggested should cover all possible Adapter async calls, and it could be implemented as follows

let run = null; // : Function[] | null

const dangerousAdapterSequence = async (params) => {
  if (run) {
    run.push(() => dangerousAdapterSequence(params));
    return;
  }
  run = [];

  await adapter.relax();
  ...  // and other chained Adapter calls, like "append", "reload" etc

  const _run = [...(run || [])];
  run = null;
  _run.forEach(call => call()); // re-invoking the reduced list of concurrent calls
};

...

dangerousAdapterSequence(params1);
dangerousAdapterSequence(params2);
dangerousAdapterSequence(params3);
...

The only Adapter thing that is fixed in this approach is the explicit "relax" call, and it makes me think that the approach could be incapsulated into the "relax" method implementation. But I need to make sure that this approach a) does really work, b) covers all related use cases, c) is the best.

safeAppend would have the same problem as current version of relax: calling "safe" methods one by one (in the same call stack) without explicit sequencing will lead to a race condition. So before thinking which part of the Lib deserves to be granted with the additional protection, we need to invent that protection. And I think if we could develop a wrapper method on the App side, that would be the first and the very important step towards the fix on the Lib side.

It reminds me of the DB transactions problem. Adapter is a kind of backend which is sensitive to concurrent calls. I agree, it should have a sort of inner protection. But if there is a suitable front-end workaround, let's start with it. How would it look like?

doAppendAndClip(items) { // : Promise<any>
  const { adapter } = this.datasource;
  return adapter.relax()
    .then(() => adapter.append(items))
    .then(() => adapter.clip());
}

onAppend(items) { // can be invoked randomly
  protectedSequence(this.doAppendAndClip, [items]);
}

protectedSequence accepts a) a function returning Promise and guaranteeing safe sub-sequence and b) an array of (a)-function arguments. It implements its own bus to provide straightforward sequence of calls of its (a)-function. I believe there must be a simple npm package for this... At last, I don't think this would be the end of the story, this is just the first idea.

[IgorVNC]

1) The main concern with dangerousAdapterSequence is that the sequence is not always defined.

It can be defferent calls at same time to different lib API, e.g.:

• append/prepend
• fix
• reload

So they can fire sporadically, means defining the particular adapter sequence is not always possible

2) Having protectedSequence is actually what I'm thinking about as well

Se we need to define a safe protectedSequence implementation, which actually will manage the realx part as well,

And then wrap all the calls inside it, semething like this:

doAppendAndClip(items) { // : Promise<any>
  const { adapter } = this.datasource;
  return adapter.append(items))
    .then(() => adapter.clip());
}

onAppend(items) { // can be invoked randomly
  protectedSequence(this.doAppendAndClip, [items]);
}

[dhilt]

@IgorVNC I was able to encapsulate the discussed protection in the Adapter.relax implementation, so we may run any number of the Adapter method sequences started with the Adapter.relax method without fear of race conditions. Please read the PR description and try ngx-ui-scroll v1.9.0-rc published on npm. "Rc" means I didn't test it perfectly, I'm planning to cover more cases with specs, and it would be great to have a feedback from you on this phase.

[IgorVNC]

Thanks @dhilt ! I will give it a try shortly

[dhilt]

@IgorVNC I released ngx-ui-scroll v1.9.0, the Adapter.relax API is considered as stable now.

[IgorVNC]

Thanks @dhilt

just one clarification - can I remove the following additional protection now?

 this.datasource.adapter.relax(() => {
          if (this.datasource.adapter.isLoading) {
            this.doScrollToBottom(); // wait a bit
            return;
          }

[dhilt]

@IgorVNC That's the goal, all extra code code should disappear as the lib internally handles unlimited sequences of the "relax" calls.