db hint

[vitaly-t]

btw, formatting query like this:

db.query(`CREATE TABLE IF NOT EXISTS ${MIGRATIONS_TABLE_NAME} (name TEXT);`);

is the same as this:

db.query("CREATE TABLE IF NOT EXISTS $1^ (name TEXT);", MIGRATIONS_TABLE_NAME);

[darkleaf]

Thanks! I know about this formatting.

How you find this repo?

[vitaly-t]

You are welcome!

GitHub search shows all new projects of your library easily, if it's name is unique enough.

[vitaly-t]

In general, this script:

  _runScriptsInParallel(paths) {
        return Promise.all(paths.map(path => this._runScript(path)));
    }

is quite inefficient, because it opens and closes a connection for every query. See explanation here: Tasks versus root/direct queries

Following that recommendation you could write:

_runScriptsInParallel(paths){
    return this.db.task(t=>t.batch(paths.map(p=>t.none(new pgp.QueryFile(p)))));
}

However, this is still not a good example, as neither is this one:

_runScript(path) {
        const script = new pgp.QueryFile(path);
        return this.db.none(script);
    }

And that's because QueryFile should really be created only once. I would suggest to refactor it so you create QueryFile only once, and then you can properly refactor the other method of executing multiple requests at once.

[darkleaf]

@vitaly-t thanks! I'll refactor it soon.

[darkleaf]

@vitaly-t спасибо за ответы, тут можно писать по-русски.

Моя библиотека должна делать следующее:

1. из заданной директории запускать sql файлы параллельно в произвольном порядке каждый в своей транзакции
2. из заданной директории запускать sql файлы последовательно в определенном порядке каждый в своей транзакции.
3. Перед исполнением применить трансформацию к содержимому файла.

Я не очень понимаю из документации как работает "Tasks" и "Synchronous Transactions". Так же непонятно зачем использовать function source(index, data, delay) вместо массива или цепочки промисов.

[vitaly-t]

"Synchronous Transactions", и function source(index, data, delay) - это все для обработки неограниченных последовательностей, что вам совсем не нужно.

Задачи - предоставляют соединение для загрузки последовательности запросов.

А по поводу остального - у вас вопросы какие-то есть?

Примеры вместе с основной документацией должны все исчерпывать.

[darkleaf]

Спасибо! Я более-менее разобрался. Если будет время, посмотрите следующую версию. Я напишу сюда, как будет готово.

[vitaly-t]

    _runScriptsInParallel(paths) {
        return this.db.task(t => {
            return Promise.all(paths.map(path => {
                return t.tx(tx => this._runScript(tx, path))
            }))
        })
    }

не используйте Promise.all в таком контексте, используйте t.batch, см. Why use method batch instead of promise.all?

[darkleaf]

@vitaly-t я правильно понимаю, что если использовать Promise.all, то если один из промисов будет отклонен, то Promise.all немедленно будет отклонен, а оставшиеся промисы продолжат выполняться? Соответственно, как только промис становится settled, pg-promise закрывает соединение?

---

Документация по Promise.all
Если одно из переданных обещаний вызывает метод reject(), вместо resolve(), то общее обещание будет отклонено.

https://github.com/vitaly-t/spex/blob/master/docs/code/batch.md но я не нашел определения promise.settle.

[vitaly-t]

'Promise.settle' - не стандартный promise, но он существует в таких библиотеках как Bluebird, например.

я правильно понимаю, что если использовать Promise.all, то если один из промисов будет отклонен, то Promise.all немедленно будет отклонен, а оставшиеся промисы продолжат выполняться?

• правильно!

Соответственно, как только промис становится settled, pg-promise закрывает соединение?

Соответсвия тут с promise.all нет, одна ошибка - он возвращает, а оставшиеся промисы продолжают выполнение. В контексте соединение это приведет к тому что оставшиеся запросы будут выполняться против закрытого уже соединения, что приведет к ошибкам. Поэтому необходим settle / завершение всех запросов перед выходом, что batch вам и предоставляет для массива промисов.

[vitaly-t]

И еще стоит учесть, что описанную проблему чрезвычайно сложно увидеть в тестовых условиях, она происходит в условиях большой нагрузки, т.к. связана с parallel race-condition.

[darkleaf]

Я понимаю это. Исправил.

Для последовательного выполнения я все правильно сделал?

    _runMigrationsSequentially(paths) {
        const initial = Promise.resolve();
        return this.db.task(t => {
            return paths.reduce((previous, path) => {
                return previous.then(() => {
                    return t.tx(tx => {
                        return Promise.resolve()
                            .then(() => this._runScript(tx, path))
                            .then(() => this._markMigrationAsPassed(tx, path))
                    })
                })
            }, initial)
        })
    }

[vitaly-t]

А зачем вы используете Promise.resolve() - он-же ничего не делает...

[darkleaf]

Он задает начало цепочки. В такой записи _runScript и _markMigrationAsPassed выглядят равнозначно и легче понять цепочку.

return Promise.resolve()
  .then(() => this._runScript(tx, path))
  .then(() => this._markMigrationAsPassed(tx, path))

return this._runScript(tx, path))
  .then(() => this._markMigrationAsPassed(tx, path))

[vitaly-t]

Это выгладит неправильно, и такой подход принадлежит к Promise Anti-Pattern.

Замените этот код:

return t.tx(tx => {
    return Promise.resolve()
        .then(() => this._runScript(tx, path))
        .then(() => this._markMigrationAsPassed(tx, path))
})

либо на это:

return t.tx(tx => {
    return this._runScript(tx, path)
        .then(() => this._markMigrationAsPassed(tx, path))
})

либо на это:

return t.tx(tx => tx.batch([
    this._runScript(tx, path),
    this._markMigrationAsPassed(tx, path)
]))

...в зависимости от того, если есть (1) или нет (2) зависимость между теми двумя запросами.

[darkleaf]

В этом конкретном случае можно использовать batch. Но я не соглашусь с таким подходом в общем.

        return Promise.resolve()
            .then(() => this.initMigrationTable())
            .then(() => this.runBeforeScripts())
            .then(() => this.runMigrations())
            .then(() => this.runAfterScripts())

Promise.resolve() начинает цепочку. Все остальные строчки равнозначны. Когда мы начинаем цепочку с вызова initMigrationTable мы как бы говорим, что initMigrationTable главный, а runBeforeScripts второстепенный.

Быстрое гугление не дало результатов, и если есть какая-то аргументация против такого подхода, то я поменяю свое мнение.

Вот пример из lodash

_([1, 2, 3])
 .tap(function(array) {
   array.pop();
 })
 .reverse()
 .value();

ПС Есть такая фраза: "Нужно не писать на языке программирования, а писать с использованием языка программирования".

[vitaly-t]

Вот тут-то как-раз вам и нужно использовать Promise.all:

return Promise.all([
    this.initMigrationTable(),
    this.runBeforeScripts(),
    this.runMigrations(),
    this.runAfterScripts()
]);

[darkleaf]

нет тут нужно последовательное выполнение

[vitaly-t]

Ха, пардон, это противоречит тому что я понял из фразы:

Все остальные строчки равнозначны.

[vitaly-t]

В этом случае правильно использовать:

return this.initMigrationTable()
    .then(() => this.runBeforeScripts())
    .then(() => this.runMigrations())
    .then(() => this.runAfterScripts())

[darkleaf]

Логически оба варианта идентичны. Мне нужно подумать над этим. У вас есть какая-то конкретная аргументация против этого подхода?

Это стилистика и этот случай напоминает https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains

[vitaly-t]

Это из теории Promise Anti-pattern, вы создаете и резолвите лишний промис ненужный.

[darkleaf]

Есть ссылка на этот материал? Я хочу разобраться в этой теме.

Я очень признателен вам за помощь, я никогда не видел, что бы автор библиотеки сам помогал с ее использованием =) Если довести ваше утверждение до абсурда, то и код не стоит разбивать на функции, т.к. каждый вызов использует стек и делает кучу лишних вещей. Все-таки код пишется в первую очередь для людей.

[vitaly-t]

Promise Anti-Patterns:

• https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns
• http://taoofcode.net/promise-anti-patterns/
• http://www.datchley.name/promise-patterns-anti-patterns/

[darkleaf]

В них нет именно моего случая. Но вот, что я нашел в http://www.datchley.name/promise-patterns-anti-patterns/

/* вместо того, что бы вручную обрабатывать ошибки и явно вызывать resolve и reject */

function makeSyncAsync() {  
  return Promise.resolve().then(function(){
    // execute synchronous code that might throw
    return value;
  });
}

// Promise returning functions to execute
function doFirstThing(){ return Promise.resolve(1); }  
function doSecondThing(res){ return Promise.resolve(res + 1); }  
function doThirdThing(res){ return Promise.resolve(res + 2); }  
function lastThing(res){ console.log("result:", res); }

var fnlist = [ doFirstThing, doSecondThing, doThirdThing, lastThing];

// Execute a list of Promise return functions in series
function pseries(list) {  
  var p = Promise.resolve();    /* <---- все начинается с этого  */
  return list.reduce(function(pacc, fn) {
    return pacc = pacc.then(fn);
  }, p);
}

pseries(fnlist);  
// result: 4

По сути, я разворачиваю reduce.

[vitaly-t]

Here: http://taoofcode.net/promise-anti-patterns/

It's a The Ghost Promise ;)