create(on: req.db) not work

[mikelin]

Steps to reproduce

Create exchangeRate records more than 120000 ,showing create successfully ,but actually records are not in psql db. As shown in the following code.

func getExchangeRates(_ req: Request) -> EventLoopFuture<ExchangeRateHistoryResult> {
        req.client.get("https://api.exchangeratesapi.io/history?start_at=2006-01-01&end_at=2020-04-01&base=USD").flatMapThrowing { res  in
            try res.content.decode(ExchangeRateHistoryResult.self)
        }.flatMap { result in
            print(result)
            let base = result.base
            result.rates?.forEach({ (key,values) in
                values.forEach { (inKey,value) in
                    let exchangeRate = ExchangeRate()
                    exchangeRate.base = base
                    exchangeRate.date = key
                    exchangeRate.target = inKey
                    exchangeRate.rate = value
                    exchangeRate.create(on: req.db).whenSuccess {
                        print("insert exchange rate:\(exchangeRate) ")
                    }
                }

            })
            return req.eventLoop.makeSucceededFuture(result)
        }

    }

Expected behavior

1、print("insert exchange rate:(exchangeRate) ") 2、new created exchange rate records actually are in psql db

Actual behavior

1、print("insert exchange rate:(exchangeRate) ") 2、 new created exchange rate records are not in psql db

Environment

• Vapor Framework version:4.0.0-rc3.11
• Vapor Toolbox version:
• OS version: macOS Catalina 10.15.4
• PSQL version: (PostgreSQL) 12.2

[VineFiner]

You can test the creation of 100 or 1000 rows, if it fails, it does not matter how much data.

[0xTim]

You're not waiting for all the rows to be saved int the database before returning from the route. Are you seeing no records in the database or some? And the database is definitely configured correctly etc? Other routes are working fine?

[mikelin]

i have tested the creation of 100 or 1000 rows ,it works. But if more than 20000 rows ,it failed. It seems to be related to the number of rows

[mikelin]

I see no records in the database after the returning from the route. If the records more than 20000 rows, it failed, if 100 or 1000 rows ,it works.

[Joannis]

I think it's because of the connection pool in SQL. SQL cannot send more than one query per connection at a time. So a pool is created with more connections for concurrent queries. Your code clearly makes a lot of parallel queries, instead of sequential or batched calls.

Maybe there's a bottleneck here in the SQL driver for these scenarios. Like a future waiting on a connection release, but it being claimed a lot as well.

[0xTim]

I wonder if you change it to not throw away the results it works. Something like

            var results = [EventLoopFuture<Void>]()
            result.rates?.forEach({ (key,values) in
                values.forEach { (inKey,value) in
                    let exchangeRate = ExchangeRate()
                    exchangeRate.base = base
                    exchangeRate.date = key
                    exchangeRate.target = inKey
                    exchangeRate.rate = value
                    results.append(exchangeRate.create(on: req.db))
                }
            })
            return results.flatten(on: req.eventLoop).transform(to: result)

[mikelin]

I wonder if you change it to not throw away the results it works. Something like

            var results = [EventLoopFuture<Void>]()
            result.rates?.forEach({ (key,values) in
                values.forEach { (inKey,value) in
                    let exchangeRate = ExchangeRate()
                    exchangeRate.base = base
                    exchangeRate.date = key
                    exchangeRate.target = inKey
                    exchangeRate.rate = value
                    results.append(exchangeRate.create(on: req.db))
                }
            })
            return results.flatten(on: req.eventLoop).transform(to: result)

I try the code , it got stuck ,seems not work.

[mikelin]

I think it's because of the connection pool in SQL. SQL cannot send more than one query per connection at a time. So a pool is created with more connections for concurrent queries. Your code clearly makes a lot of parallel queries, instead of sequential or batched calls.

Maybe there's a bottleneck here in the SQL driver for these scenarios. Like a future waiting on a connection release, but it being claimed a lot as well.

Thanks. Maybe you are right . But create(on: req.db) callback excute successfully, but data not saved .It is dangerous, i think if there's a bottleneck in the SQL driver ,should throw exception/error.

[0xTim]

There's definitely something that needs fixing

[tanner0101]

exchangeRate.create(on: req.db).whenSuccess {
    print("insert exchange rate:\(exchangeRate) ")
}

This code will only print a message on success. There needs to be a whenFailure callback in order to catch errors.

Generally I'd recommend against using forEach with futures. map + flatten is better since it's more difficult to throw away errors that way.