Analysis of problems from Odnoklassniki at JPoint 2018

    Aloha!

    Probably the most interesting event this week in the Java world was the JPoint conference , which was held at the World Trade Center in Moscow. Classmates invited visitors to also participate in the development of the most highly loaded system in Java and help our developers in solving practical problems that they face in their work.

    The eleven people who solved these problems better than anyone else have already received prizes from us, but for the rest we filmed this post with an analysis of the solutions.

    To make it more convenient for you to first try to solve the problems yourself, we hid the correct answers under the spoiler. Chur, open only after you have thought of a decision ;-)

    Let's go!

    The problem of Vadim and the score


    Vadim wrote the following code to partition music tracks on servers. What did he do wrong? Correct the error of Vadim in the code:

    /**
         * @return partition between 0 inclusive
         *         and partitions exclusive
         */intpartition(Track track, int partitions){
            assert track != null;
            assert partitions > 0;    
            return track.hashCode() % partitions;    
        }
    

    Decision
    The obvious problem in Vadim's code is that track.hashCode () can return both positive and negative values.

    Vadim is a neat person (you see how many assertes!), Which means that he also wrote the comment on the method carefully, and it says that the method will return a number between 0 and partitions exclusively.

    The obvious way to fix the problem is to have a slightly different ending to the method, for example, this:

    return Math.abs( track.hashCode() ) % partitions;
    

    Казалось бы — ура и в продакшен! Но есть одна менее очевидная проблема — по спецификации hashCode() вполне себе может вернуть и Integer.MIN_VALUE, а попытка взять от такого числа Math.abs ни к чему хорошему не приведет.

    Ох, говорила мама, следи за скобками, сынок! Правильно будет как-то так:

    return Math.abs( track.hashCode() % partitions );
    

    Другая, более серьезная проблема Вадима гораздо менее очевидна. Способ, который он выбрал, чтобы распределять данные по серверам, неконсистентен: при изменении количества партиций (например, при добавлении серверов) все треки перераспределяются по кластеру практически полностью. Если треков мало — проблем немного, а если кластер большой и треков там на сотни терабайт — их все надо переместить между серверами. А Вадим уже залил терабайты треков на сотню — другую серверов…

    Эх, Вадим, Вадим! Использовал бы ты какой-либо из вариантов консистентных хешей, не болела бы у тебя сейчас голова.

    Leonid works as a janitor


    Leonid wrote such code to clear temporary files from a directory. What did he do wrong?

    voidcleanup(Path dir)throws IOException {
            for (Path file : Files.newDirectoryStream(dir)) {
                if (file.endsWith(".tmp")) {
                    Files.delete(file);
                }
            }
        }
    

    Decision
    Леонид очень торопился выкатить очистку мусора в продакшен и невнимательно изучил документацию к DirectoryStream. DirectoryStream implements AutoCloseable. То есть имеет метод close(). А если что-то имеет такой метод — он должен вызываться. Последствия не заставили себя ждать — выкатив новую версию в продакшен, он обнаружил утечку нативной памяти в java процессе.

    Поскольку релиз включал не только этот код, источник утечки однозначно определить было нельзя, и Леониду пришлось изучить, как можно определять нативные утечки. При профилировании нашлась утечка при вызове нативного метода __alloc_dir, который как раз вызывается в результате вызова Files.newDirectoryStream. Леонид хлопнул себя по лбу и исправил проблему:

    voidcleanup(Path dir)throws IOException {
           try ( DirectoryStream<Path> stream = Files.newDirectoryStream(dir) ) {
              for (Path file : stream) {
                  if (file.endsWith(".tmp")) {
                      Files.delete(file);
                  }
              }
            }
        }
    

    — «Ура!», сказал Леонид и выкатил релиз. И снова поторопился.
    Ведь еще неплохо было бы проверить, что file из стрима является файлом, а не директорией, например, добавив условие с Files.isRegularFile(file).

    Также Леонид забыл, что Files.delete может выбросить IOException, и процесс очистки прервётся — с этим тоже надо что-то делать.

    «Зря я пошел в дворники», подумал про себя Леонид…

    Poor Dasha


    Dasha is migrating code from Java 10 to a legacy system running Java 8. What does she need to be replaced varfor the program to compile?

    var list = Arrays.asList("1", 2, 3.0);
    

    Choose the appropriate answers:

    1. List<?>
    2. List<? super Comparable>
    3. List<? extends Comparable>
    4. List<? extends Comparable & Serializable>
    5. List<? super Comparable & Serializable>

    Decision
    «Боже, на что я трачу свою жизнь», подумала Даша и быстро нашла правильные варианты: конечно же, первый, второй и третий.

    Anton, cabbage soup and limits


    Anton limits the number of requests to the face detector in user photos with the following code. How can he implement the change of the limit on the fly in a thread-safe way?

    classConcurrencyLimiter{
            staticfinalint DEFAULT_LIMIT = 10;
            final Semaphore semaphore = new Semaphore(DEFAULT_LIMIT);
            voidrunTask(Runnable task){
                semaphore.acquireUninterruptibly();
                try {
                    task.run();
                } finally {
                    semaphore.release();
                }
            }
            // Implement mevoidsetLimit(int newLimit){
                assert newLimit > 0;
                // TODO: Implementation pending
            }
        }
    

    Decision
    «Многопоточность — это сложно» — в очередной раз прочитал Антон мудрость древних с плаката и сочинил вот такой код:

    // Implementedfinal AtomicInteger limit = new AtomicInteger(DEFAULT_LIMIT);
        voidsetLimit(int newLimit){
            assert newLimit > 0;
            finalint previous = limit.getAndSet(newLimit);
            if (newLimit >= previous) {
                semaphore.release(newLimit - previous);
            } else {
                semaphore.acquireUninterruptibly(previous - newLimit);
            }
        }
    

    Естественно, что он не единственный верный, тут возможны различные вариации, например, без CAS, но с синхронизацией всего setLimit, это непринципиально.

    Но Антон не учел, что семафор-то он объявил unfair в самом начале, и попытка получить сразу много пермитов выражением semaphore.acquireUninterruptibly(previous — newLimit) может никогда не завершиться, особенно если поступает очень много задач на определение лиц, и в семафоре никогда не образуется достаточное количество пермитов за 1 раз.

    Данную проблему Антон может решить с помощью вот такого цикла, который берет пермиты по одному:

    // Implementedfinal AtomicInteger limit = new AtomicInteger(DEFAULT_LIMIT);
        voidsetLimit(int newLimit){
            assert newLimit > 0;
            finalint previous = limit.getAndSet(newLimit);
            if (newLimit >= previous) {
                semaphore.release(newLimit - previous);
            } else {
                // magic goes here:for ( int i = 0; i < previous - newLimit; i++)
                    semaphore.acquireUninterruptibly(1);
            }
        }
    

    «А может, это не лучшее решение?» — задумался Антон, ведь «Многопоточность — это сложно» — в очередной раз прочитал Антон мудрость древних с плаката висящего над его столом…

    That's all! Thank you all for taking part in solving our problems. Special thanks to those who wrote us their opinion about our tasks, came to our booth and argued with us about solutions!

    Or maybe you know the best solutions? Welcome to the comments!

    Also popular now: