Есть ли в этом коде недостаток? - PullRequest
Новая
       13

Есть ли в этом коде недостаток?

0 голосов
/ 31 мая 2011

Я написал сервлет, но сервлет еще не в стадии производства.

Я добавил счетчик в Фильтр сервлета, чтобы, когда количество одновременных запросов достигло предела, не болеелюди могут быть приняты.Я беспокоюсь о некотором предельном случае, например: скажем, система уже достигла 49 одновременных запросов (50 макс.), И в синхронизированном блоке это делает булеву переменную «ok» равной True, а затем в следующем случае несколько потоков видят, чтосервлет доступен и ворвитесь в него и нарушите лимит.

Пожалуйста, помогите проверить этот код, если есть какой-либо недостаток: 

public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
        // TODO Auto-generated method stub
        // place your code here

        // pass the request along the filter chain
        conditionalInfoLog(logEnabled, "Incoming request...");
        conditionalInfoLog(logEnabled, "Number of concurrent request(s): " + count);
        boolean ok;
        synchronized (lock) {
            ok = count < limit;
            if(ok){
                count++;
            }
        }
        if (ok) {
            try{
                // let the request through and process as usual
                conditionalInfoLog(logEnabled, "Request accepted and processing, number of concurrent request(s): " + count);
                chain.doFilter(request, response);

            }catch(Exception ex){
                conditionalErrorLog(logEnabled, ex.getMessage());
                String xmlStr = genXmlErrMsg(ex.getMessage());
                response.setContentType("text/xml");
                response.getWriter().print(xmlStr);
            }finally{
                synchronized (lock) {
                    count--;
                }
                conditionalInfoLog(logEnabled, "End of request. Number of concurrent request(s): " + count);
            }
        } else {
            // handle limit case, e.g. return status code 503
            conditionalInfoLog(logEnabled, busyMsg);
            String xmlStr = genXmlErrMsg(busyMsg);
            response.setContentType("text/xml");
            response.getWriter().print(xmlStr);
        }

    }

Ответы [ 3 ]

5 голосов
/ 31 мая 2011

Я бы лучше настроил его на уровне сервлет-контейнера, чем пытаться что-то возиться вместе. Немного приличный сервлет-контейнер тщательно протестирован и, безусловно, готов к производству.

Например, Tomcat имеет атрибут maxThreads именно для этой цели. Вы можете установить его в элементе <Connector> в server.xml. 

<Connector maxThreads="50" ...>

Это накладывает ограничение на количество одновременных запросов (по умолчанию, кстати, 200). Поэтому, когда есть 51-й запрос, он просто помещается в очередь (длина которой настраивается, кстати, атрибутом acceptCounts), пока первый не будет готов. Это также немного более удобно для пользователя, чем 503.

Смотри также:

3 голосов
/ 31 мая 2011

Это правильно, но вы в основном заново изобретаете java.util.concurrent.Semaphore: 

class MyFilter {
  final Semaphore permits = new Semaphore(50);

  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
    if (permits.tryAcquire()) {
        try {
          … // execute
        } finally {
          permits.release();
        }
    } else {
      … // handle limit case, e.g. return status code 503
    }
  }
}

Этот класс намного эффективнее, чем ваше ручное решение, и в любом случае лучше выражает ваши намерения.

0 голосов
/ 31 мая 2011

Одновременно разрешено синхронизировать не более одного потока на объекте. Если несколько потоков пытаются «ворваться» в синхронизированный блок, только один из них сделает это. Кроме того, каждый поток будет иметь свою локальную копию ok, и каждый поток должен будет пройти через этот блок: 

synchronized (lock) {
    ok = count < limit;
    if (ok) {
        count++;
    }
}

, прежде чем он сможет проверить значение своей переменной ok. Поэтому я думаю, что это довольно безопасно.

Хорошо, что вы также синхронизируете декремент count позже; это гарантирует, что значение count будет сброшено в память, прежде чем любой другой поток сможет войти в блок, который синхронизируется по lock.

...