Question about rxjava and sleeping threads

[arcadoss]

Hey!

I have a question about storio and rxjava. I try to display some info about database entries. After several subsequent requests rxjava stops produce responses from db. It ends up with ~70 sleeping threads. Here is the code.

void collectStats() {
    safeUnsubscribe(statsSubscription);

    ConnectableObservable<Event> obs = db.get()
            .listOfObjects(Event.class)
            .withQuery(DbSchema.TableEvent.QUERY_ALL_DATE_ASC)
            .prepare()
            .createObservable()
            .first()
            .flatMapIterable(events -> events)
            .replay();

    Observable<Event> eventsWithScoreObs = obs.filter(Event::scoreSaved);
    Observable<Double> scorePercentObs = eventsWithScoreObs.map(Event::getScorePercent).defaultIfEmpty(0d);

    Observable<Double> avgObs = MathObservable.from(scorePercentObs).averageDouble(aDouble -> aDouble);
    Observable<Integer> trainingsObs = obs.filter(Event::isTraining).count();
    Observable<Integer> contestObs = obs.filter(event -> !event.isTraining()).count();

    statsSubscription = Observable.zip(avgObs, trainingsObs, contestObs, C1::new)
            .subscribeOn(Schedulers.io())
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe(numbers -> {
                getUiActions().setStats(numbers.average, numbers.trainings, numbers.contests);
            });

    /*...*/
    safeSubscribe(statsSubscription); 
}

Could you explain what I'm doing wrong?

[artem-zinnatullin]

Wow, such usage of Rx operators :)

At first glance, it looks fine.

I won't be able to debug this until tonight, can you please try to replace the call to StorIO with Observable.just(fakeListOfEvents) and check if it works? Probably you just faced some deadlock in the RxJava.

[arcadoss]

I'm still learning rx and sometimes overuse it :)
It looks like problem is rxjava only and doesn't connected with StorIO.

[artem-zinnatullin]

If you're 100% sure that it's not related to StorIO — you can file an issue in RxJava https://github.com/ReactiveX/RxJava :)

Anyway, I'll try to reproduce it tonight and comment

[akarnokd]

Could you mock out the source, perhaps after the createObservable() and see if it still hangs? What are the names of the sleeping threads? Which version of RxJava are you using?

There is a bug with 1.0.14 replay that may crash/hang in some situations. No ETA on 1.0.15 which will contain the bugfix.

[arcadoss]

I find out what caused the problem. It was StorIO v1.2.0 :) After updating to 1.4.0 everything works as expected. With the outdated version the following code wouldn't update UI more than twice.

void collectStats() {
    if (statsSubscription != null) {
        statsSubscription.unsubscribe();
    }

    Subscription statsSubscription = db.get()
            .listOfObjects(Event.class)
            .withQuery(DbSchema.TableEvent.QUERY_ALL_DATE_ASC)
            .prepare()
            .createObservable()
            .first()
            .flatMapIterable(events -> events)
            .reduce(0d, (aDouble, event) -> aDouble + event.getScorePercent())
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe(aDouble -> {
                getUiActions().setStats(aDouble, 0, 0);
            });
}

[artem-zinnatullin]

@arcadoss ah, looks like it was a bug with deadlock that we've fixed in StorIO 1.2.1!

For some reason I was sure that you're using latest StorIO 1.4.0 and forgot to ask you version, my bad, sorry :(

@akarnokd everything is fine on the RxJava side, thanks for your attention :)

[artem-zinnatullin]

@arcadoss if you don't mind, can you please share some feedback: what are your feelings about StorIO? What do you like and what would you like us to improve? :)

[dimsuz]

I wasn't invited, but can I still share? :) Been using it a bit more, so have some things on my mind.

Actually, one thing. StorIO is good at knowing how to fill my types with info from DB. But I find it a bit tedious that I still have to specify table each time I create a query to run. I know that query can be complex and involve several tables, but it would be very convenient if there was some way to only specify 'where' parts when creating a query object, i.e. it would be cool if this lib could auto-deduce table for my type. I don't know if this makes sense, I just have a feeling of verbosity: I must mention same exact table when creating get/put/delete resolvers and then also with every query for every type I create a mapping for.

[artem-zinnatullin]

I wasn't invited, but can I still share? :)

Sure!

Got it… But I guess, we can not do it automatically simply because:

• You can store same objects in multiple tables.
• StorIO core has no idea about mapping YourType -> tables, I guess you use sqlite-annotations, but since they are separate module, we can not combine core with this module too much…

Sorry for that :(

But, I think you can do this in user-space:

class YourTypeTable {

  @NonNull
  public static Query queryBuilder() {
    return Query.builder()
      .table("your_type"); // And probably other default params if you need to share them between all queries
  }
}

And then use it like this

storIOSQLite
  .get()
  .listOfObjects(YourType.class)
  .withQuery(YourTypeTable.queryBuilder()
    .where("someColumn = ?")
    .whereArgs(someValue)
    .build())
  ...

// We mostly use StorIOContentResolver and there you can see full power of Query that does not know about mapping between YourType and Uri, because usually we have multiple Uris per Type.

Thanks for the feedback @dimsuz!

[arcadoss]

@artem-zinnatullin I think it's a great library and i enjoy using it. However I noticed a couple of things that I don't understand.

First of all I didn't find a way to make fields in db model final while keeping automatically generated resolvers. It looks like it is possible to create annotation to specify builders (something similar to lombok).

I noticed that mapFromCursor method in GetResolver class have cursor object as single argument while other methods have storio instace as another argument. In order to read domain model object I need to query multiple tables and it looks like the best place to do it is inside mapFromCursor method. But absence of storio instance in arguments make me feel that I'm doing something wrong. I ended with custom get resolver that looks like this

public class MyGetResolver extends GetResolver<Event> {
    private StorIOSQLite storIOSQLite;

    @NonNull @Override public Event mapFromCursor(Cursor cursor) {
        Event event = new Event();
        /* ... */
        if (otherID != 0) {
            List<Bla> bla = storIOSQLite.get().listOfObjects(Bla.class)
                    .withQuery(Q).prepare().executeAsBlocking();
            event.bla = bla;
        }
        return event;
    }

    @NonNull @Override public Cursor performGet(@NonNull StorIOSQLite storIOSQLite, @NonNull RawQuery rawQuery) {
        this.storIOSQLite = storIOSQLite;
        return super.performGet(storIOSQLite, rawQuery);
    }
}

Plain text table creation query looks messy in code. To make it a little bit prettier I wrote StringBuilder wrapper. It would be nice to have at least something like this inside your library.

 MetaHelper.newTable(sb, NAME)
                .column(_ID, "INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT")
                .period()
                .foreignKey(_ID_BLA, BLA.NAME, BLA._ID)
                .period()
                .end();

Get operation returns stream of lists. It's usefull if you want to recieve updates, but in case when you interested in single value you should call take(1).flatMapIterable() . But I don't think you should add aditional method to shorten this call. The absence of It annoyed me at first, but now I think one would need single value only when application is not reactive enought :)

[artem-zinnatullin]

First of all I didn't find a way to make fields in db model final while keeping automatically generated resolvers. It looks like it is possible to create annotation to specify builders (something similar to lombok).

Yep, it's possible to do in the future, probably we will even add support for AutoValue out of the box!

I noticed that mapFromCursor method in GetResolver class have cursor object as single argument while other methods have storio instace as another argument. In order to read domain model object I need to query multiple tables and it looks like the best place to do it is inside mapFromCursor method. But absence of storio instance in arguments make me feel that I'm doing something wrong. I ended with custom get resolver that looks like this

Yeah, it's my personal mistake in the API… Sorry for that :( Here is the issue tracking this change #519.

Plain text table creation query looks messy in code. To make it a little bit prettier I wrote StringBuilder wrapper. It would be nice to have at least something like this inside your library.

We want to avoid any kinds of SQL generation in the StorIO because it's not very trivial thing to do and StorIO is not ORM. Feel free to release this generator as a separate library and if we will find it good — we will recommend it directly in our README!

Get operation returns stream of lists. It's usefull if you want to recieve updates, but in case when you interested in single value you should call take(1).flatMapIterable() . But I don't think you should add aditional method to shorten this call. The absence of It annoyed me at first, but now I think one would need single value only when application is not reactive enought :)

We want to add method for retrieving single value from the storage, here is the issue #329 :)

@arcadoss thank you very much for such detailed feedback, we will keep working on StorIO to make it better!

[Rainer-Lang]

+1000000000000 for "AutoValue out of the box"-support

[artem-zinnatullin]

Okay :)

[dimsuz]

+1000 for autovalue from me too. I tried to add support to the current AnnotationProcessor, but I failed. Or should I say, temporarily suspended my efforts :) I have never written an AnnotationProcessor so far and got stuck because putting some resolver-related annotations on @AutoValue-ed class methods makes those annotations appear in two places: 1) in abstract class holding @AutoValue 2) in the class generated by auto-value. So StorIO's annotation processor should be able to handle this somehow.

And also I am not sure if it is actually a good idea to add the support for auto-value to the current annotation processor or create another one, or refactor the processor code somehow to be more extensible. Because looking at it seems there are a lot of things which could be reused.

[dimsuz]

Perhaps this should be discussed in a separate issue. I'm not sure when I will be able to look again, so not creating it.

[artem-zinnatullin]

Created issue for AutoValue #528

[dimsuz]

Another piece of feedback.
db.get().<...>.createObservable() creates an observable which will watch for db changes. Oftentimes I use this function only to make asynchronous read without the need to be listening for updates. But because 'listening'-version is default, I need to be cautious and remember to stick .take(1) after each read-once-and-be-done createObservable(). And as long as you need to remember something, you will forget this. So maybe it would be nice to have a 'Single' variant of createObservable() or to have it accept some flag to govern this behavior.

Again, not sure if this is something that should be put in the library, or if client should be the one who creates some utility method to manage this.

[arcadoss]

@dimsuz there is the issue for that feature #329

[dimsuz]

@arcadoss thanks, subscribed

[artem-zinnatullin]

@arcadoss @dimsuz #329 is about retrieving one object instead of List, but it will be same reactive as get().listOfObjects() (imagine you need to constantly update some view with info from one object).

We are thinking about Single support, added issue for that #529!