[GitHub] flink pull request #6019: [FLINK-9182]async checkpoints for timer service

[makeyang <gi...@git.apache.org>]

GitHub user makeyang opened a pull request:

    https://github.com/apache/flink/pull/6019

    [FLINK-9182]async checkpoints for timer service

    ## What is the purpose of the change
    
    it is for async checkpoints for timer service
    the whole idea is based on discussion in previous PR for FLINK-9182 in this link:https://github.com/apache/flink/pull/5908
    
    ## Brief change log
    
    in sync part flat copy of the internal array of the priority queue
    in async part build key group and write timer key group
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
      - The serializers: (no)
      - The runtime per-record code paths (performance sensitive): (no)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
      - The S3 file system connector: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/makeyang/flink FLINK-9182-version2

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/6019.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #6019
    
----
commit 82799922203bd6cb959c11336f71aee4def431d7
Author: makeyang <ma...@...>
Date:   2018-05-16T05:44:16Z

    [FLINK-9182]async checkpoints for timer service
    the whole idea is based on discussion on github: https://github.com/apache/flink/pull/5908
    the idea is propesed by StefanRRichter as below:
    "Second, I would probably suggest a simpler model for the async snapshots. You dropped the idea of making flat copies, but I wonder if this was premature. I can see that calling set.toArray(...) per keygroup could (maybe) turn out a bit slow because it has to potentially iterate and flatten linked entries. However, with async snapshots, we could get rid of the key-group partitioning of sets, and instead do a flat copy of the internal array of the priority queue. This would translate to just a single memcopy call internally, which is very efficient. In the async part, we can still partition the timers by key-group in a similar way as the copy-on-write state table does. This would avoid slowing down the event processing path (in fact improving it be unifying the sets) and also keep the approach very straight forward and less invasive."

----


---

[GitHub] flink issue #6019: [FLINK-9182]async checkpoints for timer service

[StefanRRichter <gi...@git.apache.org>]

Github user StefanRRichter commented on the issue:

    https://github.com/apache/flink/pull/6019
  
    @sihuazhou my plan was to integrate the timer service more closely with the keyed state backends, starting from the point that we are merging the PR for timers in RocksDB. I think timers should just be considered as keyed state and eventually become part of the keyed state backend's snapshot.
    
    @makeyang with the above comment, that essentially means that with the work on the RocksDB timer service we are planning to have a larger rewrite of how snapshotting for timer works anyways. I have also concrete plans and work for doing this for the heap timer service, as well as including a efficient way to delete timers. All this unfortunately does not match too well with this PR. I suggest that this can be closed because we will address this issue for the next release in somewhat more holistic way.


---

[GitHub] flink issue #6019: [FLINK-9182]async checkpoints for timer service

[sihuazhou <gi...@git.apache.org>]

Github user sihuazhou commented on the issue:

    https://github.com/apache/flink/pull/6019
  
    @StefanRRichter I definitely agree with your point! The timers should be considered as keyed state is a beautiful way to go!


---

[GitHub] flink pull request #6019: [FLINK-9182]async checkpoints for timer service

[makeyang <gi...@git.apache.org>]

Github user makeyang closed the pull request at:

    https://github.com/apache/flink/pull/6019


---

[GitHub] flink issue #6019: [FLINK-9182]async checkpoints for timer service

[makeyang <gi...@git.apache.org>]

Github user makeyang commented on the issue:

    https://github.com/apache/flink/pull/6019
  
    @StefanRRichter & @sihuazhou thanks u guys


---

[GitHub] flink issue #6019: [FLINK-9182]async checkpoints for timer service

[sihuazhou <gi...@git.apache.org>]

Github user sihuazhou commented on the issue:

    https://github.com/apache/flink/pull/6019
  
    I wonder can you introduce a `HeapState` which scoped to `key group` to support timer service. This way timer service is backed by keyed state backend, which looks like a beautiful things.


---

[GitHub] flink issue #6019: [FLINK-9182]async checkpoints for timer service

[StefanRRichter <gi...@git.apache.org>]

Github user StefanRRichter commented on the issue:

    https://github.com/apache/flink/pull/6019
  
    Yes, the only reason I did not start with this is that I first wanted to wait for the completion and merge of the RocksDB timer service to have a complete picture.


---