aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mehrdad Nurolahzade <mehr...@nurolahzade.com>
Subject Re: Review Request 55105: AURORA-1870 Add finer grained timings to the Snapshot process
Date Thu, 05 Jan 2017 04:36:13 GMT


> On Jan. 4, 2017, 10:48 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java, lines
520-532
> > <https://reviews.apache.org/r/55105/diff/1/?file=1594663#file1594663line520>
> >
> >     Can these be default methods on the interface rather than converting to an abstract
class?
> 
> Mehrdad Nurolahzade wrote:
>     You are right, will refactor.

Actually, converting it back to an interface causes `SnapshotStoreImplIT` fail after `Stats`
is flushed; a static field is only initialized once.
```
private interface SnapshotField {

    LoadingCache<String, SlidingStats> STATS = CacheBuilder.newBuilder().build(
        new CacheLoader<String, SlidingStats>() {
          @Override
          public SlidingStats load(String name) throws Exception {
            return new SlidingStats(name, "nanos");
          }
        });

    String getName();

    void saveToSnapshot(MutableStoreProvider storeProvider, Snapshot snapshot);

    void restoreFromSnapshot(MutableStoreProvider storeProvider, Snapshot snapshot);

    default void save(MutableStoreProvider storeProvider, Snapshot snapshot) {
      STATS.getUnchecked(SNAPSHOT_SAVE + getName())
          .time((Timeable.NoResult.Quiet) () -> saveToSnapshot(storeProvider, snapshot));
    }

    default void restore(MutableStoreProvider storeProvider, Snapshot snapshot) {
      STATS.getUnchecked(SNAPSHOT_RESTORE + getName())
          .time((Timeable.NoResult.Quiet) () -> restoreFromSnapshot(storeProvider, snapshot));
    }
  }
```
Seems like it is safer to keep it as abstract class?


- Mehrdad


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55105/#review160521
-----------------------------------------------------------


On Jan. 4, 2017, 8:36 p.m., Mehrdad Nurolahzade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55105/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2017, 8:36 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1870
>     https://issues.apache.org/jira/browse/AURORA-1870
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1870 Add finer grained timings to the Snapshot process
> 
> I gave up on `@Timed` interceptor approach because major refactoring is required in order
to have snapshot fields instantiated by Guice through `Provider` or `@Provides` interfaces.
The abstract class approach is much easier/cleaner.
> 
> 
> Diffs
> -----
> 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingStats.java f7a5ae41e307627fc55157758e9b7cdd861c3268

>   commons/src/test/java/org/apache/aurora/common/stats/SlidingStatsTest.java PRE-CREATION

>   src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 7aa111ec14696ae40f518c42f3c7f45d8ab0e94c

>   src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java f56a1624c7188da175ad3e6de323c3442802f2ea

> 
> Diff: https://reviews.apache.org/r/55105/diff/
> 
> 
> Testing
> -------
> 
> ```
> $ curl localhost:8081/vars | grep snapshot_restore
>   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
>                                  Dload  Upload   Total   Spent    Left  Speed
>   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
> snapshot_restore_crons_events 1
> snapshot_restore_crons_events_per_sec 0.0
> snapshot_restore_crons_nanos_per_event 0.0
> snapshot_restore_crons_nanos_total 73648
> snapshot_restore_crons_nanos_total_per_sec 0.0
> snapshot_restore_dbscript_events 1
> snapshot_restore_dbscript_events_per_sec 0.0
> snapshot_restore_dbscript_nanos_per_event 0.0
> snapshot_restore_dbscript_nanos_total 1148842021
> snapshot_restore_dbscript_nanos_total_per_sec 0.0
> snapshot_restore_hosts_events 1
> snapshot_restore_hosts_events_per_sec 0.0
> snapshot_restore_hosts_nanos_per_event 0.0
> snapshot_restore_hosts_nanos_total 76166
> snapshot_restore_hosts_nanos_total_per_sec 0.0
> snapshot_restore_job_updates_events 1
> snapshot_restore_job_updates_events_per_sec 0.0
> snapshot_restore_job_updates_nanos_per_event 0.0
> snapshot_restore_job_updates_nanos_total 49482
> snapshot_restore_job_updates_nanos_total_per_sec 0.0
> snapshot_restore_locks_events 1
> snapshot_restore_locks_events_per_sec 0.0
> snapshot_restore_locks_nanos_per_event 0.0
> snapshot_restore_locks_nanos_total 125084
> snapshot_restore_locks_nanos_total_per_sec 0.0
> snapshot_restore_quota_events 1
> snapshot_restore_quota_events_per_sec 0.0
> snapshot_restore_quota_nanos_per_event 0.0
> snapshot_restore_quota_nanos_total 52305
> snapshot_restore_quota_nanos_total_per_sec 0.0
> snapshot_restore_scheduler_metadata_events 1
> snapshot_restore_scheduler_metadata_events_per_sec 0.0
> snapshot_restore_scheduler_metadata_nanos_per_event 0.0
> snapshot_restore_scheduler_metadata_nanos_total 70816
> snapshot_restore_scheduler_metadata_nanos_total_per_sec 0.0
> snapshot_restore_tasks_events 1
> snapshot_restore_tasks_events_per_sec 0.0
> snapshot_restore_tasks_nanos_per_event 0.0
> snapshot_restore_tasks_nanos_total 91377
> snapshot_restore_tasks_nanos_total_per_sec 0.0
> ```
> 
> ```
> $ aurora_admin scheduler_snapshot devcluster
>  INFO] Response from scheduler: OK (message: )
>  
> $ curl localhost:8081/vars | grep snapshot_save
>   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
>                                  Dload  Upload   Total   Spent    Left  Speed
> 100 48226    0 48226    0     0  3266k      0 --:--:-- --:--:-- --:--:-- 3363k
> snapshot_save_crons_events 1
> snapshot_save_crons_events_per_sec 0.0
> snapshot_save_crons_nanos_per_event 0.0
> snapshot_save_crons_nanos_total 466181
> snapshot_save_crons_nanos_total_per_sec 0.0
> snapshot_save_dbscript_events 1
> snapshot_save_dbscript_events_per_sec 0.0
> snapshot_save_dbscript_nanos_per_event 0.0
> snapshot_save_dbscript_nanos_total 18201542
> snapshot_save_dbscript_nanos_total_per_sec 0.0
> snapshot_save_hosts_events 1
> snapshot_save_hosts_events_per_sec 0.0
> snapshot_save_hosts_nanos_per_event 0.0
> snapshot_save_hosts_nanos_total 1286180
> snapshot_save_hosts_nanos_total_per_sec 0.0
> snapshot_save_job_updates_events 1
> snapshot_save_job_updates_events_per_sec 0.0
> snapshot_save_job_updates_nanos_per_event 0.0
> snapshot_save_job_updates_nanos_total 123760632
> snapshot_save_job_updates_nanos_total_per_sec 0.0
> snapshot_save_locks_events 1
> snapshot_save_locks_events_per_sec 0.0
> snapshot_save_locks_nanos_per_event 0.0
> snapshot_save_locks_nanos_total 1523926
> snapshot_save_locks_nanos_total_per_sec 0.0
> snapshot_save_quota_events 1
> snapshot_save_quota_events_per_sec 0.0
> snapshot_save_quota_nanos_per_event 0.0
> snapshot_save_quota_nanos_total 5725489
> snapshot_save_quota_nanos_total_per_sec 0.0
> snapshot_save_scheduler_metadata_events 1
> snapshot_save_scheduler_metadata_events_per_sec 0.0
> snapshot_save_scheduler_metadata_nanos_per_event 0.0
> snapshot_save_scheduler_metadata_nanos_total 2142
> snapshot_save_scheduler_metadata_nanos_total_per_sec 0.0
> snapshot_save_tasks_events 1
> snapshot_save_tasks_events_per_sec 0.0
> snapshot_save_tasks_nanos_per_event 0.0
> snapshot_save_tasks_nanos_total 6939463
> snapshot_save_tasks_nanos_total_per_sec 0.0 
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message